apurtell commented on code in PR #2540:
URL: https://github.com/apache/phoenix/pull/2540#discussion_r3500567968
##########
phoenix-core/src/test/java/org/apache/phoenix/replication/ReplicationLogGroupTest.java:
##########
@@ -1429,59 +1456,15 @@ public void testRetryPicksUpStagedWriter() throws
Exception {
assertTrue("Should be using new writer", newWriter != initialWriter);
// Old writer: received the append only
- verify(initialWriter, times(1)).append(eq(tableName), eq(commitId),
eq(put));
+ verify(initialWriter, times(1)).append(eq(tableName), eq(commitId),
+ eq(LogFileTestUtil.cellsOf(put)));
// New writer: received replayed append + successful sync
- verify(newWriter, times(1)).append(eq(tableName), eq(commitId), eq(put));
+ verify(newWriter, times(1)).append(eq(tableName), eq(commitId),
+ eq(LogFileTestUtil.cellsOf(put)));
verify(newWriter, times(1)).sync();
}
- /**
- * Tests the idle-then-lease-recovery scenario: after a sync clears
currentBatch, the system goes
- * idle. A rotation tick stages a pending writer. The reader performs HDFS
lease recovery,
- * breaking the old writer's stream. When events resume, apply() drains the
healthy staged writer
- * before the action — the broken writer is never touched. No replay needed
(empty batch).
- */
- @Test
- public void testIdleLeaseRecoveryDrainsStagedWriter() throws Exception {
Review Comment:
I think this test is a real concurrency concern. After a sync clears
`currentBatch`, the system goes idle, a rotation tick stages a pending writer,
invalidating the old writer. When events resume we need to ensure `apply()`
drains to the new and healthy writer before the action and should ensure the
dead writer is never touched.
The body of this test only needs some simple updates (any(Mutation.class) →
any(List.class), eq(put) → eq(LogFileTestUtil.cellsOf(put))) Let's restore and
migrate it.
--
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]
