Davis-Zhang-Onehouse commented on code in PR #19717:
URL: https://github.com/apache/hudi/pull/19717#discussion_r3874850055
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metadata/HoodieBackedTableMetadataWriter.java:
##########
@@ -435,8 +453,16 @@ private void initializeMetadataPartition(
Indexer indexer,
String dataTableInstantTime,
Map<String, List<FileInfo>> partitionToAllFilesMap,
- Lazy<List<FileSliceAndPartition>> lazyMergedFileSlices) throws
IOException {
- String instantTimeForPartition =
generateUniqueInstantTime(dataTableInstantTime);
+ Lazy<List<FileSliceAndPartition>> lazyMergedFileSlices,
+ Option<String> requestedIndexPartition) throws IOException {
+ // A requested partition initializes under a fresh solo-family instant,
never the indexing
+ // action's own instant. The action's completion applies its data commit
to the metadata table
+ // too, and finding that instant already completed there is treated as a
partially applied
+ // earlier commit: it is rolled back and re-applied, destroying the
initialization records
+ // while leaving the file groups. The solo family is the established shape
for
+ // metadata-table-only bootstrap commits and survives that reconciliation.
+ String instantTimeForPartition = requestedIndexPartition.isPresent()
+ ? generateUniqueSoloInstantTime() :
generateUniqueInstantTime(dataTableInstantTime);
Review Comment:
The wipe needs two metadata-table commits at the same instant time:
commitInternal treats a completed MDT commit at the incoming instant as a
re-attempt, rolls it back and re-applies. generateUniqueInstantTime returns the
action's own instant as-is for every initialization inside an indexing action,
with no uniqueness loop, so the other types share the exposure, but only when a
single action performs more than one initialization: each
initializeMetadataPartition call in that action gets the same instant, and the
second commit destroys the first partition's bulk-inserted records while its
file groups remain. A single-type action commits once at that instant and is
safe, which is the shape the existing RECORD_INDEX and COLUMN_STATS indexer
tests exercise, and a genuinely re-attempted action recommitting the same
partition at the same instant is the case that rollback-and-recommit exists to
heal.
I scoped the solo instant to the requested secondary and expression
partitions because this PR is what makes those initializations reachable under
an action at all, and because moving the other types off the action instant is
a behavior change with its own blast radius: the HUDI-5733 handling in
BaseHoodieTableServiceClient exempts inflight MDT deltacommits at indexing
instants from eager rollback, which a solo-instant init no longer matches (it
instead behaves like the regular-write bootstrap commits, which already use the
solo family), and it changes the re-attempt semantics above. Extending the solo
family to every initialization an indexing action performs looks right to me,
but as a deliberate follow-up; happy to file an issue for it.
##########
hudi-utilities/src/test/java/org/apache/hudi/utilities/TestHoodieIndexer.java:
##########
@@ -184,6 +188,49 @@ public void testIndexerForRecordIndex() {
/**
* Test indexer for RLI and secondary index.
*/
+ /**
+ * An indexing action builds the partition it names. With two
secondary-index definitions registered and neither
+ * initialized (the shape of a table whose metadata table was rebuilt with
its definitions intact), the action for
+ * one of them builds exactly that one, on storage and not only in the table
config.
+ */
+ @Test
+ void
testIndexerBuildsTheRequestedSecondaryIndexWhenSeveralAreUninitialized() {
Review Comment:
Done. The test is now
testIndexerBuildsEachRequestedIndexAmongSeveralUninitialized: two
secondary-index and two expression-index definitions registered up front (the
rebuilt-metadata-table shape), all four built with consecutive actions,
asserting after each action that only the requested partition appeared, in the
table config and on storage, and at the end that every built partition has file
slices. The branches an indexer run cannot reach directly are unit-tested in
TestSecondaryIndexer with the requested-partition context: the requested
partition among several uninitialized definitions, the already-initialized
skip, and the unresolvable-request failure.
--
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]
