New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[FLINK-9609] [connectors] Add bucket ready mechanism for BucketingSin… #6375
Conversation
f958949
to
3c9780f
Compare
} | ||
} | ||
|
||
@Override | ||
public boolean isBucketReady(Set<Path> bucketPathes) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems this implementation of the method has logic problem? shall we return true;
after iterating the for loop and remove the return true;
above the catch
?
Is the purpose of the method for checking all the bucketPathes never end with pending or inProgress suffix?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we finish the for loop and haven't returned true yet, then return false. I think this is reasonable. If a file end with pending or inProgress, This means that the file is not ready. and we can not use it.
Hi, @tillrohrmann Could you please take a look on this PR ? Actually, I don't know which committer is more familiar with |
return false; | ||
} | ||
} | ||
return true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean this return statement, can not verify all the bucket path is ready, right? because the loop is not finished.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahhh. Oops! Yes, you are very right! :)
…k when checkpoint complete
3c9780f
to
32dd05e
Compare
The BucketingSink has been removed. |
The lazy review of the community wastes the huge enthusiasm and energy of early contributors. This is the reason for the decline or the homogenization of the community. |
What is the purpose of the change
Currently, BucketingSink only support
notifyCheckpointComplete
. However, users want to do some extra work when a bucket is ready. It would be nice if we can support BucketReady mechanism for users or we can tell users when a bucket is ready for use. For example, One bucket is created for every 5 minutes, at the end of 5 minutes before creating the next bucket, the user might need to do something as the previous bucket ready, like sending the timestamp of the bucket ready time to a server or do some other stuff.Here, Bucket ready means all the part files suffix name under a bucket neither .pending nor .in-progress. Then we can think this bucket is ready for user use. Like a watermark means no elements with a timestamp older or equal to the watermark timestamp should arrive at the window. We can also refer to the concept of watermark here, or we can call this BucketWatermark if we could.
Brief change log
Add an interface
BucketReady
.Verifying this change
This change is a trivial rework / code cleanup without any test coverage.
Does this pull request potentially affect one of the following parts:
@Public(Evolving)
: (no)Documentation