the-other-tim-brown commented on code in PR #13255:
URL: https://github.com/apache/hudi/pull/13255#discussion_r2072412006
##########
hudi-client/hudi-client-common/src/test/java/org/apache/hudi/client/transaction/TestSimpleConcurrentFileWritesConflictResolutionStrategy.java:
##########
@@ -115,32 +116,50 @@ public void
testConcurrentWritesWithInterleavingSuccessfulCommit() throws Except
List<HoodieInstant> candidateInstants =
strategy.getCandidateInstants(metaClient, currentInstant.get(),
lastSuccessfulInstant).collect(
Collectors.toList());
// writer 1 conflicts with writer 2
- Assertions.assertTrue(candidateInstants.size() == 1);
+ Assertions.assertEquals(1, candidateInstants.size());
ConcurrentOperation thatCommitOperation = new
ConcurrentOperation(candidateInstants.get(0), metaClient);
ConcurrentOperation thisCommitOperation = new
ConcurrentOperation(currentInstant.get(), currentMetadata);
Assertions.assertTrue(strategy.hasConflict(thisCommitOperation,
thatCommitOperation));
- try {
- strategy.resolveConflict(null, thisCommitOperation, thatCommitOperation);
- Assertions.fail("Cannot reach here, writer 1 and writer 2 should have
thrown a conflict");
- } catch (HoodieWriteConflictException e) {
- // expected
- }
+ Assertions.assertThrows(HoodieWriteConflictException.class, () ->
strategy.resolveConflict(null, thisCommitOperation, thatCommitOperation));
}
@Test
public void testConcurrentWritesWithReplaceInflightCommit() throws Exception
{
-
TestConflictResolutionStrategyUtil.createClusterInflight(metaClient.createNewInstantTime(),
metaClient);
- HoodieActiveTimeline timeline = metaClient.getActiveTimeline();
+ String currentWriterInstant = metaClient.createNewInstantTime();
+ createInflightCommit(currentWriterInstant, metaClient);
+ Option<HoodieInstant> currentInstant =
Option.of(INSTANT_GENERATOR.createNewInstant(State.INFLIGHT,
HoodieTimeline.COMMIT_ACTION, currentWriterInstant));
+
+ String replaceInstant = metaClient.createNewInstantTime();
+ HoodieTestTable.of(metaClient).addRequestedReplace(replaceInstant,
Option.empty());
+ TestConflictResolutionStrategyUtil.createReplaceInflight(replaceInstant,
metaClient);
Review Comment:
Previously this test was using clustering instead of the replace-commit, so
I have updated it to match the test naming
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/SimpleConcurrentFileWritesConflictResolutionStrategy.java:
##########
@@ -67,8 +68,7 @@ public Stream<HoodieInstant>
getCandidateInstants(HoodieTableMetaClient metaClie
Stream<HoodieInstant> compactionAndClusteringPendingTimeline =
activeTimeline
.filterPendingReplaceClusteringAndCompactionTimeline()
.filter(instant -> ClusteringUtils.isClusteringInstant(activeTimeline,
instant, metaClient.getInstantGenerator())
- || HoodieTimeline.COMPACTION_ACTION.equals(instant.getAction()))
- .findInstantsAfter(currentInstant.requestedTime())
+ || (!HoodieTimeline.CLUSTERING_ACTION.equals(instant.getAction())
&& compareTimestamps(instant.requestedTime(), GREATER_THAN,
currentInstant.requestedTime())))
Review Comment:
For clustering instants, we want to include any pending clustering. This
prevents you from accidentally skipping over a clustering commit that started
before this commit. For the compaction and insert-overwrite we only need to
consider the commits that started after this one.
--
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]
