stevenzwu commented on pull request #13784:
URL: https://github.com/apache/flink/pull/13784#issuecomment-716864714


   BTW, noticed that the jira number is wrong. FLINK-19689 was for a different 
issue.
   
   I like Stephan's points above. 
   
   - `Checkpointable` seems unnecessary considering almost all sources need 
some checkpointing in streaming mode.
   - reusing `CheckpointListener` for `onCheckpointComplete(checkpointId)` 
seems like a reasonable option.
   - `snapshotState(checkpointId)` is better than two broken-up methods of 
`snapshotState()` and `onCheckpoint(checkpointId)`,  because either way 
checkpointId is leaked to source implementation. I don't see much problem with 
leaking checkpointId, because we are not talking about end users of Kafka 
source.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to