fhan688 commented on PR #3503:
URL: https://github.com/apache/fluss/pull/3503#issuecomment-4762246552
> @fhan688 Thanks for the PR! I spent quite a while trying to understand the
purpose of the `tag` field, and I have some concerns about this approach.
>
> The current `tag` feels a bit hacky and hard to follow. It's a free-form
`String` that overloads two meanings via the `"firstSplit-"` prefix plus a
timestamp, and the only way to interpret it is by string-parsing the prefix. I
can imagine the Hudi writer will then have to parse this `tag` string as well
to figure out the first split, which ends up being fairly hard-coded.
>
> I'm not very familiar with Hudi, so please correct me if I'm wrong. But
based on FIP-24, my understanding of what we actually need is:
>
> > the Hudi writer needs to know which split is the first split, so that
only the `LakeWriter` handling the first split generates the global
`instantTime`, and the other writers retrieve it via `CkpMetadata`.
>
> If that's the case, instead of encoding this into a string `tag`, what if
we introduce a typed field `splitIndex` in `TieringSplit` representing the
index of the current split within this tiering round? It pairs naturally with
the existing `numberOfSplits` (`splitIndex` / `numberOfSplits`, similar to
Flink's `getIndexOfThisSubtask()` / `getNumberOfParallelSubtasks()`), so it
reads much less weird. Then the coordinator is simply the writer whose split
has `splitIndex == 0`, and that writer generates the global `instantTime`.
>
> This avoids the string parsing / hard-coding, makes the split immutable
(no `setTag()`), and the "exactly one first split" property becomes structural
(contiguous indices ⇒ exactly one `0`).
Thanks for the detailed suggestion. I agree that overloading `tag` with both
the first-split marker and a timestamp makes the protocol hard to follow.
I double-checked the Hudi tiering writer side. Besides identifying the
writer that creates the global Hudi instant, the writer also needs a
round-level timestamp to avoid consuming a stale pending instant from
`CkpMetadata`. In the current Hudi writer flow, non-first writers wait for
`CkpMetadata.lastPendingInstant()`, and compare it with the split-generated
timestamp to make sure the pending instant belongs to the current
tiering round. So `splitIndex` alone can cover the first-writer election,
but it cannot replace the round timestamp semantics.
I’ll update the design to make these two meanings explicit instead of
encoding them into a free-form string:
- introduce a typed `splitIndex` in `TieringSplit`, where `splitIndex ==
0` identifies the writer responsible for creating the Hudi instant;
- introduce an explicit round timestamp field, for example
`tieringRoundTimestamp`, to let Hudi writers validate the pending instant from
`CkpMetadata`;
- remove the `"firstSplit-"` string parsing and avoid mutable `setTag()`.
This should keep the first-split property structural while preserving the
timestamp requirement needed by the Hudi writer.
--
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]
