hudi-agent commented on code in PR #18789:
URL: https://github.com/apache/hudi/pull/18789#discussion_r3272995925
##########
hudi-flink-datasource/hudi-flink/src/main/java/org/apache/hudi/sink/StreamWriteOperatorCoordinator.java:
##########
@@ -759,7 +759,7 @@ private static class TableState implements Serializable {
final boolean syncMetadata;
final boolean isDeltaTimeCompaction;
final boolean isStreamingIndexWriteEnabled;
- final boolean isRLIWithBootstrap;
+ final boolean isRecordLevelIndex;
Review Comment:
π€ nit: `isRecordLevelIndex` here shadows the name of
`OptionsResolver.isRecordLevelIndex()`, which only covers the non-global
variant β yet this field is `true` for *either* type. A reader seeing
`tableState.isRecordLevelIndex` might assume it has the same scope as the
resolver method. Something like `isAnyRecordLevelIndex` (or even just a brief
Javadoc on the field) would make the "global OR non-global" intent explicit.
<sub><i>- AI-generated; verify before applying. React π/π to flag
quality.</i></sub>
##########
hudi-flink-datasource/hudi-flink/src/main/java/org/apache/hudi/sink/StreamWriteOperatorCoordinator.java:
##########
@@ -565,7 +565,7 @@ private void handleBootstrapEvent(WriteMetadataEvent event)
{
if (eventBuffer.allBootstrapEventsReceived()) {
// start to recommit the instant.
boolean committed = recommitInstant(event.getCheckpointId(),
event.getInstantTime(), eventBuffer);
- if (committed && tableState.isRLIWithBootstrap) {
+ if (committed && tableState.isRecordLevelIndex) {
Review Comment:
π€ The old guard required `INDEX_BOOTSTRAP_ENABLED=true`, so an
RLIBootstrapOperator (or BootstrapOperator) was guaranteed to be in the
pipeline when this `failJob` fired. With the new `isRecordLevelIndex` flag,
this branch also runs when `index.bootstrap.enabled=false` β but per
`Pipelines.streamBootstrap`, no `index_bootstrap` operator is wired into the
graph in that case. Could you confirm whether the global failover is still
desired/needed when there is no bootstrap operator to reload state? If yes, the
exception message ("so that RLI bootstrap operator can load the record level
index completely") is misleading; if no, the condition may need to keep the
`INDEX_BOOTSTRAP_ENABLED` check (or be replaced with one that reflects when
index state must actually be re-bootstrapped). @danny0405 could you weigh in on
the intended scope here?
<sub><i>- AI-generated; verify before applying. React π/π to flag
quality.</i></sub>
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
