[hudi-9041]send act commit event when reuse current instant #12849
Conversation
…ent instant that recovered from checkpoint
|
cc @cshuo to take care of it. |
|
@gejinzh thks for contributing. Please format the commit message, generally, the title should be "[HUDI-XXX] xxx". Refer to https://hudi.apache.org/contribute/developer-setup/#contributing-code for detail. |
| @@ -449,6 +449,10 @@ private void handleBootstrapEvent(WriteMetadataEvent event) { | |||
| LOG.warn("Reuse current pending Instant {} with {} operationType, " | |||
| + "ignoring empty bootstrap event.", this.instant, WriteOperationType.INSERT.value()); | |||
| reset(); | |||
| if (event.isRestored()) { | |||
There was a problem hiding this comment.
IIUC, event is definitely restored from state in this branch, as this.instant is a valid instant. So it's ok to just send ack directly without checking? Correctly me if I miss something.
There was a problem hiding this comment.
The write task will only be unblocked when creating an instant or sending an ack event;
This code can only be executed when reusing instant.
When reusing instant, it indicates that there is an inflight instant equal to this.instant and no new instant will be created;
Therefore, when the event is restored from the state, we need an ack event to unblock the write tasks;
I don't know if I have answered your question,I'm not getting the question very well
There was a problem hiding this comment.
@gejinzh My point is that it's unnecessary to check the condition event.isRestored(), and we can simple sendCommitAckEvents(-1L); here.
There was a problem hiding this comment.
If there is no data flush in append mode, instant will also be reused here; And the coordinator will send an act event at notifyCheckpointComplete; So without this condition, two act events will be sent;
There was a problem hiding this comment.
@gejinzh Could you please also add another test to cover the case you described?
There was a problem hiding this comment.
So without this condition, two act events will be sent;
It should be okay because they are 2 different commits.
|
@gejinzh BTW, it's better to add a test for the case. Here is test patch, you can apply it by |
github-actions
bot
added
size:M
gejinzh
changed the title
|
+1. cc @danny0405 |
|
@gejinzh Can you fix the compile error: Error: /home/runner/work/hudi/hudi/hudi-flink-datasource/hudi-flink/src/test/java/org/apache/hudi/sink/utils/MockSubtaskGateway.java:[44,30] cannot find symbol |
gejinzh
changed the title
…stant that recovered from checkpoint
Change Logs
Send commit ack event when reuse instant that recovered from checkpoint
Impact
Send commit ack event when reuse instant that recovered from checkpoint to unblock write tasks
Risk level (write none, low medium or high below)
low
Documentation Update
none
Contributor's checklist