Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.

...

Please keep the discussion on the mailing list rather than commenting on the wiki (wiki discussions get unwieldy fast).

Motivation

...

Why is it necessary to make SingleThreadFetcherManager PublicEvolving?

  1. Though the SingleThreadFetcherManager is annotated as Internal, it actually acts as some-degree public API, which is widely used in many connector projects:

...

  1. flink-

...

  1. connector-

...

  1. kafka,  flink-connector-mongodb, flink-cdc-connectors and soon. In flink-connector-kafka, KafkaSourceFetcherManager even extends SingleThreadFetcherManager.
  2. More over, even the constructor of SingleThreadMultiplexSourceReaderBase(which is PublicEvolving) includes the params of SingleThreadFetcherManager   and FutureCompletingBlockingQueue. That means that the SingleThreadFetcherManager

...

  1. and FutureCompletingBlockingQueue  have already been exposed to users for a long time and are widely

...

As shown in FLINK-31324, FLINK-28853 used to change the default constructor of SingleThreadFetcherManager.However, it influenced a lot. Finally, the former constructor was added back and marked asDeprecated。

Therefore, why not make SingleThreadFetcherManager PublicEvolving?

Let's go back to more contexts from the origin design: 

...

  1. used.As shown in FLINK-31324, FLINK-28853 used to change the default constructor of SingleThreadFetcherManager.However, it influenced a lot. Finally, the former constructor was added back and marked asDeprecated.

    Code Block
    public SingleThreadMultiplexSourceReaderBase(
        FutureCompletingBlockingQueue<RecordsWithSplitIds<E>> elementsQueue,
        SingleThreadFetcherManager<E, SplitT> splitFetcherManager,
        RecordEmitter<E, T, SplitStateT> recordEmitter,
        Configuration config,
        SourceReaderContext context)
    { super(elementsQueue, splitFetcherManager, recordEmitter, config, context); }

...

  1. 
    
    



  1. The original design of SplitFetcherManager and its subclasses was to make them public to the Source developers. The goal is to let us take care of the threading model, while the Source developers can just focus on the SplitReader implementation. Therefore, I think making SplitFetcherManater / SingleThreadFetcherManager public aligns with the original design. That is also why these classes are exposed in the constructor of SourceReaderBase.


2. For  For FutureCompletingBlockingQueue, as a hindsight, it might be better to not expose it to the Source developers. They are unlikely to use it anywhere other than just constructing it. The reason that FutureCompletingBlockingQueue is currently exposed in the SourceReaderBase constructor is because both the SplitFetcherManager and SourceReaderBase need it. One way to hide the FutureCompletingBlockingQueue from the public API is to make SplitFetcherManager the only owner class of the queue, and expose some of its methods via SplitFetcherManager. This way, the SourceReaderBase can invoke the methods via SplitFetcherManager. I believe this also makes the code slightly cleaner.

...