You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "JAkutenshi (via GitHub)" <gi...@apache.org> on 2025/02/20 20:07:25 UTC
[PR] IGNITE-22620 Implement ReplicaSafeTimeSyncRequest processing for zone replica [ignite-3]
JAkutenshi opened a new pull request, #5258:
URL: https://github.com/apache/ignite-3/pull/5258
JIRA Ticket: [IGNITE-22620](https://issues.apache.org/jira/browse/IGNITE-22620)
## The goal
## The reason
## The solution
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22620 Implement ReplicaSafeTimeSyncRequest processing for zone replica [ignite-3]
Posted by "JAkutenshi (via GitHub)" <gi...@apache.org>.
JAkutenshi commented on code in PR #5258:
URL: https://github.com/apache/ignite-3/pull/5258#discussion_r1969771743
##########
modules/partition-replicator/src/integrationTest/java/org/apache/ignite/internal/partition/replicator/ItReplicaLifecycleTest.java:
##########
@@ -656,6 +660,41 @@ public void testScanCloseReplicaRequest() throws Exception {
assertDoesNotThrow(tx::commit);
}
+ @Test
+ public void testReplicaSafeTimeSyncRequest() throws Exception {
+ // Prepare a single node cluster.
+ startCluster(2);
+ Node node0 = getNode(0);
+ List<Set<Assignment>> assignments = PartitionDistributionUtils.calculateAssignments(
+ cluster.stream().map(n -> n.name).collect(toList()), 1, 1);
+
+ List<TokenizedAssignments> tokenizedAssignments = assignments.stream()
+ .map(a -> new TokenizedAssignmentsImpl(a, Integer.MIN_VALUE))
+ .collect(toList());
+
+ placementDriver.setPrimary(node0.clusterService.topologyService().localMember());
+ placementDriver.setAssignments(tokenizedAssignments);
+
+ // Prepare a zone.
+ String zoneName = "test_zone";
+ createZone(node0, zoneName, 1, 2);
+ int zoneId = DistributionZonesTestUtil.getZoneId(node0.catalogManager, zoneName, node0.hybridClock.nowLong());
+ int partId = 0;
+
+ // Create a table to work with.
+ createTable(node0, zoneName, "test_table");
+
+ HybridTimestamp node0safeTimeBefore = node0.currentSafeTimeForZonePartition(zoneId, partId);
Review Comment:
But seems it isn't enough
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22620 Implement ReplicaSafeTimeSyncRequest processing for zone replica [ignite-3]
Posted by "sashapolo (via GitHub)" <gi...@apache.org>.
sashapolo commented on code in PR #5258:
URL: https://github.com/apache/ignite-3/pull/5258#discussion_r1981775427
##########
modules/partition-replicator/src/main/java/org/apache/ignite/internal/partition/replicator/raft/ZonePartitionRaftListener.java:
##########
@@ -266,9 +270,16 @@ private IgniteBiTuple<Serializable, Boolean> processCrossTableProcessorsCommand(
long commandTerm,
@Nullable HybridTimestamp safeTimestamp
) {
+ if (tableProcessors.isEmpty()) {
+ synchronized (tableProcessorsStateLock) {
+ return new IgniteBiTuple<>(null, lastAppliedIndex < commandIndex);
+ }
+ }
+
IgniteBiTuple<Serializable, Boolean> result = new IgniteBiTuple<>(null, false);
tableProcessors.values().forEach(processor -> {
+
Review Comment:
?
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22620 Implement ReplicaSafeTimeSyncRequest processing for zone replica [ignite-3]
Posted by "sanpwc (via GitHub)" <gi...@apache.org>.
sanpwc commented on code in PR #5258:
URL: https://github.com/apache/ignite-3/pull/5258#discussion_r1965818292
##########
modules/partition-replicator/src/integrationTest/java/org/apache/ignite/internal/partition/replicator/ItReplicaLifecycleTest.java:
##########
@@ -656,6 +660,41 @@ public void testScanCloseReplicaRequest() throws Exception {
assertDoesNotThrow(tx::commit);
}
+ @Test
+ public void testReplicaSafeTimeSyncRequest() throws Exception {
+ // Prepare a single node cluster.
+ startCluster(2);
+ Node node0 = getNode(0);
+ List<Set<Assignment>> assignments = PartitionDistributionUtils.calculateAssignments(
+ cluster.stream().map(n -> n.name).collect(toList()), 1, 1);
+
+ List<TokenizedAssignments> tokenizedAssignments = assignments.stream()
+ .map(a -> new TokenizedAssignmentsImpl(a, Integer.MIN_VALUE))
+ .collect(toList());
+
+ placementDriver.setPrimary(node0.clusterService.topologyService().localMember());
+ placementDriver.setAssignments(tokenizedAssignments);
+
+ // Prepare a zone.
+ String zoneName = "test_zone";
+ createZone(node0, zoneName, 1, 2);
+ int zoneId = DistributionZonesTestUtil.getZoneId(node0.catalogManager, zoneName, node0.hybridClock.nowLong());
+ int partId = 0;
+
+ // Create a table to work with.
+ createTable(node0, zoneName, "test_table");
+
+ HybridTimestamp node0safeTimeBefore = node0.currentSafeTimeForZonePartition(zoneId, partId);
Review Comment:
currentSafeTimeForZonePartition may fail with` Missing resources for zone` if you will call
```
ZonePartitionResources getZonePartitionResources(ZonePartitionId zonePartitionId) {
ZoneResources zoneResources = resourcesByZoneId.get(zonePartitionId.zoneId());
```
fast enough.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22620 Implement ReplicaSafeTimeSyncRequest processing for zone replica [ignite-3]
Posted by "JAkutenshi (via GitHub)" <gi...@apache.org>.
JAkutenshi commented on code in PR #5258:
URL: https://github.com/apache/ignite-3/pull/5258#discussion_r1981369121
##########
modules/partition-replicator/src/integrationTest/java/org/apache/ignite/internal/partition/replicator/ItReplicaLifecycleTest.java:
##########
@@ -636,6 +639,54 @@ public void testScanCloseReplicaRequest() throws Exception {
assertDoesNotThrow(tx::commit);
}
+ @Test
+ public void testReplicaSafeTimeSyncRequest() throws Exception {
+ startCluster(2);
+ Node node = getNode(0);
+
+ // Prepare a zone.
+ String zoneName = "test_zone";
+ createZone(node, zoneName, 1, 2);
+ int zoneId = DistributionZonesTestUtil.getZoneId(node.catalogManager, zoneName, node.hybridClock.nowLong());
+ int partId = 0;
+
+ // Wait for sure that zone replication group was created on both nodes.
+ waitForZoneReplicaStartedOnNode(node, zoneId, partId);
+ waitForZoneReplicaStartedOnNode(getNode(1), zoneId, partId);
+
+ // Check that time was adjusted even without table.
+ checkSafeTimeWasAdjustedForZoneGroup(zoneId, partId);
+
+ // Create a table to work with.
+ String tableName = "test_table";
+ createTable(node, zoneName, tableName);
+ assertDoesNotThrow(() -> TableTestUtils.getTableIdStrict(node.catalogManager, tableName, node.hybridClock.nowLong()));
Review Comment:
Because we should check that time is adjusted for both cases: there is a table in the zone or not.
##########
modules/partition-replicator/src/integrationTest/java/org/apache/ignite/internal/partition/replicator/ItReplicaLifecycleTest.java:
##########
@@ -636,6 +639,54 @@ public void testScanCloseReplicaRequest() throws Exception {
assertDoesNotThrow(tx::commit);
}
+ @Test
+ public void testReplicaSafeTimeSyncRequest() throws Exception {
+ startCluster(2);
+ Node node = getNode(0);
+
+ // Prepare a zone.
+ String zoneName = "test_zone";
+ createZone(node, zoneName, 1, 2);
+ int zoneId = DistributionZonesTestUtil.getZoneId(node.catalogManager, zoneName, node.hybridClock.nowLong());
+ int partId = 0;
+
+ // Wait for sure that zone replication group was created on both nodes.
+ waitForZoneReplicaStartedOnNode(node, zoneId, partId);
+ waitForZoneReplicaStartedOnNode(getNode(1), zoneId, partId);
+
+ // Check that time was adjusted even without table.
+ checkSafeTimeWasAdjustedForZoneGroup(zoneId, partId);
+
+ // Create a table to work with.
+ String tableName = "test_table";
+ createTable(node, zoneName, tableName);
+ assertDoesNotThrow(() -> TableTestUtils.getTableIdStrict(node.catalogManager, tableName, node.hybridClock.nowLong()));
+
+ // Check that time was adjusted table.
Review Comment:
Fixed.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22620 Implement ReplicaSafeTimeSyncRequest processing for zone replica [ignite-3]
Posted by "JAkutenshi (via GitHub)" <gi...@apache.org>.
JAkutenshi commented on code in PR #5258:
URL: https://github.com/apache/ignite-3/pull/5258#discussion_r1979078164
##########
modules/partition-replicator/src/main/java/org/apache/ignite/internal/partition/replicator/raft/ZonePartitionRaftListener.java:
##########
@@ -239,7 +243,9 @@ private IgniteBiTuple<Serializable, Boolean> processCrossTableProcessorsCommand(
}
});
- return result;
+ return tableProcessors.isEmpty()
Review Comment:
Done
##########
modules/partition-replicator/src/integrationTest/java/org/apache/ignite/internal/partition/replicator/ItReplicaLifecycleTest.java:
##########
@@ -658,6 +662,47 @@ public void testScanCloseReplicaRequest() throws Exception {
assertDoesNotThrow(tx::commit);
}
+ @Test
+ public void testReplicaSafeTimeSyncRequest() throws Exception {
+ startCluster(2);
+ Node node = getNode(0);
+
+ placementDriver.setPrimary(node.clusterService.topologyService().localMember());
+
+ // Prepare a zone.
+ String zoneName = "test_zone";
+ createZone(node, zoneName, 1, 2);
+ int zoneId = assertDoesNotThrow(() -> DistributionZonesTestUtil.getZoneIdStrict(
Review Comment:
Fixed
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22620 Implement ReplicaSafeTimeSyncRequest processing for zone replica [ignite-3]
Posted by "sanpwc (via GitHub)" <gi...@apache.org>.
sanpwc commented on code in PR #5258:
URL: https://github.com/apache/ignite-3/pull/5258#discussion_r1965793699
##########
modules/partition-replicator/src/integrationTest/java/org/apache/ignite/internal/partition/replicator/ItReplicaLifecycleTest.java:
##########
@@ -656,6 +660,41 @@ public void testScanCloseReplicaRequest() throws Exception {
assertDoesNotThrow(tx::commit);
}
+ @Test
+ public void testReplicaSafeTimeSyncRequest() throws Exception {
+ // Prepare a single node cluster.
Review Comment:
It's not a single node cluster))
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22620 Implement ReplicaSafeTimeSyncRequest processing for zone replica [ignite-3]
Posted by "JAkutenshi (via GitHub)" <gi...@apache.org>.
JAkutenshi commented on code in PR #5258:
URL: https://github.com/apache/ignite-3/pull/5258#discussion_r1969970150
##########
modules/partition-replicator/src/integrationTest/java/org/apache/ignite/internal/partition/replicator/ItReplicaLifecycleTest.java:
##########
@@ -658,6 +662,47 @@ public void testScanCloseReplicaRequest() throws Exception {
assertDoesNotThrow(tx::commit);
}
+ @Test
+ public void testReplicaSafeTimeSyncRequest() throws Exception {
+ startCluster(2);
+ Node node = getNode(0);
+
+ placementDriver.setPrimary(node.clusterService.topologyService().localMember());
+
+ // Prepare a zone.
+ String zoneName = "test_zone";
+ createZone(node, zoneName, 1, 2);
+ int zoneId = assertDoesNotThrow(() -> DistributionZonesTestUtil.getZoneIdStrict(
Review Comment:
I assumed somewhy that `*Strict` methods help me with complete zone/table assertion. Foolish idea, I see now
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22620 Implement ReplicaSafeTimeSyncRequest processing for zone replica [ignite-3]
Posted by "JAkutenshi (via GitHub)" <gi...@apache.org>.
JAkutenshi commented on code in PR #5258:
URL: https://github.com/apache/ignite-3/pull/5258#discussion_r1981365448
##########
modules/partition-replicator/src/main/java/org/apache/ignite/internal/partition/replicator/raft/ZonePartitionRaftListener.java:
##########
@@ -266,6 +270,10 @@ private IgniteBiTuple<Serializable, Boolean> processCrossTableProcessorsCommand(
long commandTerm,
@Nullable HybridTimestamp safeTimestamp
) {
+ if (tableProcessors.isEmpty()) {
+ return new IgniteBiTuple<>(null, true);
Review Comment:
Fixed as we discussed yesterday: we should check the local in-memory `lastAppliedIndex` and if it lower than the received -- update index & term and then adjust the time.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22620 Implement ReplicaSafeTimeSyncRequest processing for zone replica [ignite-3]
Posted by "sashapolo (via GitHub)" <gi...@apache.org>.
sashapolo commented on code in PR #5258:
URL: https://github.com/apache/ignite-3/pull/5258#discussion_r1981516074
##########
modules/partition-replicator/src/main/java/org/apache/ignite/internal/partition/replicator/raft/ZonePartitionRaftListener.java:
##########
@@ -268,13 +272,24 @@ private IgniteBiTuple<Serializable, Boolean> processCrossTableProcessorsCommand(
) {
IgniteBiTuple<Serializable, Boolean> result = new IgniteBiTuple<>(null, false);
- tableProcessors.values().forEach(processor -> {
- IgniteBiTuple<Serializable, Boolean> r = processor.processCommand(command, commandIndex, commandTerm, safeTimestamp);
- // Need to adjust the safe time if any of the table processors successfully handled the command.
- if (Boolean.TRUE.equals(r.get2())) {
- result.set2(Boolean.TRUE);
+ if (tableProcessors.isEmpty()) {
+ synchronized (tableProcessorsStateLock) {
+ if (lastAppliedIndex < commandIndex) {
+ lastAppliedIndex = commandIndex;
+ lastAppliedTerm = commandTerm;
+
+ result.set2(Boolean.TRUE);
Review Comment:
Can be replaced with just `return new IgniteBiTuple<>(null, lastAppliedIndex < commandIndex)`;
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22620 Implement ReplicaSafeTimeSyncRequest processing for zone replica [ignite-3]
Posted by "JAkutenshi (via GitHub)" <gi...@apache.org>.
JAkutenshi commented on code in PR #5258:
URL: https://github.com/apache/ignite-3/pull/5258#discussion_r1969721773
##########
modules/partition-replicator/src/integrationTest/java/org/apache/ignite/internal/partition/replicator/ItReplicaLifecycleTest.java:
##########
@@ -656,6 +660,41 @@ public void testScanCloseReplicaRequest() throws Exception {
assertDoesNotThrow(tx::commit);
}
+ @Test
+ public void testReplicaSafeTimeSyncRequest() throws Exception {
+ // Prepare a single node cluster.
+ startCluster(2);
+ Node node0 = getNode(0);
+ List<Set<Assignment>> assignments = PartitionDistributionUtils.calculateAssignments(
+ cluster.stream().map(n -> n.name).collect(toList()), 1, 1);
+
+ List<TokenizedAssignments> tokenizedAssignments = assignments.stream()
+ .map(a -> new TokenizedAssignmentsImpl(a, Integer.MIN_VALUE))
+ .collect(toList());
+
+ placementDriver.setPrimary(node0.clusterService.topologyService().localMember());
+ placementDriver.setAssignments(tokenizedAssignments);
+
+ // Prepare a zone.
+ String zoneName = "test_zone";
+ createZone(node0, zoneName, 1, 2);
+ int zoneId = DistributionZonesTestUtil.getZoneId(node0.catalogManager, zoneName, node0.hybridClock.nowLong());
+ int partId = 0;
+
+ // Create a table to work with.
+ createTable(node0, zoneName, "test_table");
+
+ HybridTimestamp node0safeTimeBefore = node0.currentSafeTimeForZonePartition(zoneId, partId);
Review Comment:
Now I'm asserting zone and table creation explicitly
##########
modules/partition-replicator/src/integrationTest/java/org/apache/ignite/internal/partition/replicator/ItReplicaLifecycleTest.java:
##########
@@ -656,6 +660,41 @@ public void testScanCloseReplicaRequest() throws Exception {
assertDoesNotThrow(tx::commit);
}
+ @Test
+ public void testReplicaSafeTimeSyncRequest() throws Exception {
+ // Prepare a single node cluster.
+ startCluster(2);
+ Node node0 = getNode(0);
+ List<Set<Assignment>> assignments = PartitionDistributionUtils.calculateAssignments(
+ cluster.stream().map(n -> n.name).collect(toList()), 1, 1);
+
+ List<TokenizedAssignments> tokenizedAssignments = assignments.stream()
+ .map(a -> new TokenizedAssignmentsImpl(a, Integer.MIN_VALUE))
+ .collect(toList());
+
+ placementDriver.setPrimary(node0.clusterService.topologyService().localMember());
+ placementDriver.setAssignments(tokenizedAssignments);
+
+ // Prepare a zone.
+ String zoneName = "test_zone";
+ createZone(node0, zoneName, 1, 2);
+ int zoneId = DistributionZonesTestUtil.getZoneId(node0.catalogManager, zoneName, node0.hybridClock.nowLong());
+ int partId = 0;
+
+ // Create a table to work with.
+ createTable(node0, zoneName, "test_table");
Review Comment:
Done
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22620 Implement ReplicaSafeTimeSyncRequest processing for zone replica [ignite-3]
Posted by "sashapolo (via GitHub)" <gi...@apache.org>.
sashapolo commented on code in PR #5258:
URL: https://github.com/apache/ignite-3/pull/5258#discussion_r1979530156
##########
modules/partition-replicator/src/main/java/org/apache/ignite/internal/partition/replicator/raft/ZonePartitionRaftListener.java:
##########
@@ -266,6 +270,10 @@ private IgniteBiTuple<Serializable, Boolean> processCrossTableProcessorsCommand(
long commandTerm,
@Nullable HybridTimestamp safeTimestamp
) {
+ if (tableProcessors.isEmpty()) {
+ return new IgniteBiTuple<>(null, true);
Review Comment:
Shouldn't it be `false` instead? And I think this check is redundant anyway.
##########
modules/partition-replicator/src/integrationTest/java/org/apache/ignite/internal/partition/replicator/ItReplicaLifecycleTest.java:
##########
@@ -636,6 +639,54 @@ public void testScanCloseReplicaRequest() throws Exception {
assertDoesNotThrow(tx::commit);
}
+ @Test
+ public void testReplicaSafeTimeSyncRequest() throws Exception {
+ startCluster(2);
+ Node node = getNode(0);
+
+ // Prepare a zone.
+ String zoneName = "test_zone";
+ createZone(node, zoneName, 1, 2);
+ int zoneId = DistributionZonesTestUtil.getZoneId(node.catalogManager, zoneName, node.hybridClock.nowLong());
+ int partId = 0;
+
+ // Wait for sure that zone replication group was created on both nodes.
+ waitForZoneReplicaStartedOnNode(node, zoneId, partId);
+ waitForZoneReplicaStartedOnNode(getNode(1), zoneId, partId);
Review Comment:
Would it be enough to call `waitForMetadataCompletenessAtNow` on all nodes instead?
##########
modules/partition-replicator/src/integrationTest/java/org/apache/ignite/internal/partition/replicator/ItReplicaLifecycleTest.java:
##########
@@ -636,6 +639,54 @@ public void testScanCloseReplicaRequest() throws Exception {
assertDoesNotThrow(tx::commit);
}
+ @Test
+ public void testReplicaSafeTimeSyncRequest() throws Exception {
+ startCluster(2);
+ Node node = getNode(0);
+
+ // Prepare a zone.
+ String zoneName = "test_zone";
+ createZone(node, zoneName, 1, 2);
+ int zoneId = DistributionZonesTestUtil.getZoneId(node.catalogManager, zoneName, node.hybridClock.nowLong());
+ int partId = 0;
+
+ // Wait for sure that zone replication group was created on both nodes.
+ waitForZoneReplicaStartedOnNode(node, zoneId, partId);
+ waitForZoneReplicaStartedOnNode(getNode(1), zoneId, partId);
+
+ // Check that time was adjusted even without table.
+ checkSafeTimeWasAdjustedForZoneGroup(zoneId, partId);
+
+ // Create a table to work with.
+ String tableName = "test_table";
+ createTable(node, zoneName, tableName);
+ assertDoesNotThrow(() -> TableTestUtils.getTableIdStrict(node.catalogManager, tableName, node.hybridClock.nowLong()));
+
+ // Check that time was adjusted table.
+ checkSafeTimeWasAdjustedForZoneGroup(zoneId, partId);
+ }
+
+ private static void waitForZoneReplicaStartedOnNode(Node node, int zoneId, int partId) throws InterruptedException {
+ ZonePartitionId zoneReplicationId = new ZonePartitionId(zoneId, partId);
+
+ waitForCondition(() -> node.replicaManager.replica(zoneReplicationId) != null, AWAIT_TIMEOUT_MILLIS);
Review Comment:
missing `assertTrue`
##########
modules/partition-replicator/src/integrationTest/java/org/apache/ignite/internal/partition/replicator/ItReplicaLifecycleTest.java:
##########
@@ -636,6 +639,54 @@ public void testScanCloseReplicaRequest() throws Exception {
assertDoesNotThrow(tx::commit);
}
+ @Test
+ public void testReplicaSafeTimeSyncRequest() throws Exception {
+ startCluster(2);
+ Node node = getNode(0);
+
+ // Prepare a zone.
+ String zoneName = "test_zone";
+ createZone(node, zoneName, 1, 2);
+ int zoneId = DistributionZonesTestUtil.getZoneId(node.catalogManager, zoneName, node.hybridClock.nowLong());
+ int partId = 0;
+
+ // Wait for sure that zone replication group was created on both nodes.
+ waitForZoneReplicaStartedOnNode(node, zoneId, partId);
+ waitForZoneReplicaStartedOnNode(getNode(1), zoneId, partId);
+
+ // Check that time was adjusted even without table.
+ checkSafeTimeWasAdjustedForZoneGroup(zoneId, partId);
+
+ // Create a table to work with.
+ String tableName = "test_table";
+ createTable(node, zoneName, tableName);
+ assertDoesNotThrow(() -> TableTestUtils.getTableIdStrict(node.catalogManager, tableName, node.hybridClock.nowLong()));
+
+ // Check that time was adjusted table.
+ checkSafeTimeWasAdjustedForZoneGroup(zoneId, partId);
+ }
+
+ private static void waitForZoneReplicaStartedOnNode(Node node, int zoneId, int partId) throws InterruptedException {
+ ZonePartitionId zoneReplicationId = new ZonePartitionId(zoneId, partId);
+
+ waitForCondition(() -> node.replicaManager.replica(zoneReplicationId) != null, AWAIT_TIMEOUT_MILLIS);
+ assertThat(node.replicaManager.replica(zoneReplicationId), willCompleteSuccessfully());
+ }
+
+ private void checkSafeTimeWasAdjustedForZoneGroup(int zoneId, int partId) throws InterruptedException {
Review Comment:
Can we generalize this method on a per-node basis? It looks strangely specific that we check two nodes inside.
##########
modules/partition-replicator/src/integrationTest/java/org/apache/ignite/internal/partition/replicator/ItReplicaLifecycleTest.java:
##########
@@ -636,6 +639,54 @@ public void testScanCloseReplicaRequest() throws Exception {
assertDoesNotThrow(tx::commit);
}
+ @Test
+ public void testReplicaSafeTimeSyncRequest() throws Exception {
+ startCluster(2);
+ Node node = getNode(0);
+
+ // Prepare a zone.
+ String zoneName = "test_zone";
+ createZone(node, zoneName, 1, 2);
+ int zoneId = DistributionZonesTestUtil.getZoneId(node.catalogManager, zoneName, node.hybridClock.nowLong());
+ int partId = 0;
+
+ // Wait for sure that zone replication group was created on both nodes.
+ waitForZoneReplicaStartedOnNode(node, zoneId, partId);
+ waitForZoneReplicaStartedOnNode(getNode(1), zoneId, partId);
+
+ // Check that time was adjusted even without table.
+ checkSafeTimeWasAdjustedForZoneGroup(zoneId, partId);
+
+ // Create a table to work with.
+ String tableName = "test_table";
+ createTable(node, zoneName, tableName);
+ assertDoesNotThrow(() -> TableTestUtils.getTableIdStrict(node.catalogManager, tableName, node.hybridClock.nowLong()));
+
+ // Check that time was adjusted table.
+ checkSafeTimeWasAdjustedForZoneGroup(zoneId, partId);
+ }
+
+ private static void waitForZoneReplicaStartedOnNode(Node node, int zoneId, int partId) throws InterruptedException {
+ ZonePartitionId zoneReplicationId = new ZonePartitionId(zoneId, partId);
+
+ waitForCondition(() -> node.replicaManager.replica(zoneReplicationId) != null, AWAIT_TIMEOUT_MILLIS);
+ assertThat(node.replicaManager.replica(zoneReplicationId), willCompleteSuccessfully());
+ }
+
+ private void checkSafeTimeWasAdjustedForZoneGroup(int zoneId, int partId) throws InterruptedException {
+ Node node0 = getNode(0);
+ Node node1 = getNode(1);
+
+ HybridTimestamp node0safeTimeBefore = node0.currentSafeTimeForZonePartition(zoneId, partId);
+ HybridTimestamp node1safeTimeBefore = node1.currentSafeTimeForZonePartition(zoneId, partId);
+
+ waitForCondition(
Review Comment:
missing `assertTrue`
##########
modules/partition-replicator/src/integrationTest/java/org/apache/ignite/internal/partition/replicator/ItReplicaLifecycleTest.java:
##########
@@ -636,6 +639,54 @@ public void testScanCloseReplicaRequest() throws Exception {
assertDoesNotThrow(tx::commit);
}
+ @Test
+ public void testReplicaSafeTimeSyncRequest() throws Exception {
+ startCluster(2);
+ Node node = getNode(0);
+
+ // Prepare a zone.
+ String zoneName = "test_zone";
+ createZone(node, zoneName, 1, 2);
+ int zoneId = DistributionZonesTestUtil.getZoneId(node.catalogManager, zoneName, node.hybridClock.nowLong());
+ int partId = 0;
+
+ // Wait for sure that zone replication group was created on both nodes.
+ waitForZoneReplicaStartedOnNode(node, zoneId, partId);
+ waitForZoneReplicaStartedOnNode(getNode(1), zoneId, partId);
+
+ // Check that time was adjusted even without table.
+ checkSafeTimeWasAdjustedForZoneGroup(zoneId, partId);
+
+ // Create a table to work with.
+ String tableName = "test_table";
+ createTable(node, zoneName, tableName);
+ assertDoesNotThrow(() -> TableTestUtils.getTableIdStrict(node.catalogManager, tableName, node.hybridClock.nowLong()));
Review Comment:
Why is this needed?
##########
modules/partition-replicator/src/integrationTest/java/org/apache/ignite/internal/partition/replicator/ItReplicaLifecycleTest.java:
##########
@@ -636,6 +639,54 @@ public void testScanCloseReplicaRequest() throws Exception {
assertDoesNotThrow(tx::commit);
}
+ @Test
+ public void testReplicaSafeTimeSyncRequest() throws Exception {
+ startCluster(2);
+ Node node = getNode(0);
+
+ // Prepare a zone.
+ String zoneName = "test_zone";
+ createZone(node, zoneName, 1, 2);
+ int zoneId = DistributionZonesTestUtil.getZoneId(node.catalogManager, zoneName, node.hybridClock.nowLong());
+ int partId = 0;
+
+ // Wait for sure that zone replication group was created on both nodes.
+ waitForZoneReplicaStartedOnNode(node, zoneId, partId);
+ waitForZoneReplicaStartedOnNode(getNode(1), zoneId, partId);
+
+ // Check that time was adjusted even without table.
+ checkSafeTimeWasAdjustedForZoneGroup(zoneId, partId);
+
+ // Create a table to work with.
+ String tableName = "test_table";
+ createTable(node, zoneName, tableName);
+ assertDoesNotThrow(() -> TableTestUtils.getTableIdStrict(node.catalogManager, tableName, node.hybridClock.nowLong()));
+
+ // Check that time was adjusted table.
Review Comment:
Something is not right with this sentence
##########
modules/partition-replicator/src/integrationTest/java/org/apache/ignite/internal/partition/replicator/ItReplicaLifecycleTest.java:
##########
@@ -636,6 +639,54 @@ public void testScanCloseReplicaRequest() throws Exception {
assertDoesNotThrow(tx::commit);
}
+ @Test
+ public void testReplicaSafeTimeSyncRequest() throws Exception {
+ startCluster(2);
+ Node node = getNode(0);
+
+ // Prepare a zone.
+ String zoneName = "test_zone";
+ createZone(node, zoneName, 1, 2);
+ int zoneId = DistributionZonesTestUtil.getZoneId(node.catalogManager, zoneName, node.hybridClock.nowLong());
+ int partId = 0;
+
+ // Wait for sure that zone replication group was created on both nodes.
+ waitForZoneReplicaStartedOnNode(node, zoneId, partId);
+ waitForZoneReplicaStartedOnNode(getNode(1), zoneId, partId);
+
+ // Check that time was adjusted even without table.
Review Comment:
```suggestion
// Check that time was adjusted even without tables.
```
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22620 Implement ReplicaSafeTimeSyncRequest processing for zone replica [ignite-3]
Posted by "sashapolo (via GitHub)" <gi...@apache.org>.
sashapolo commented on code in PR #5258:
URL: https://github.com/apache/ignite-3/pull/5258#discussion_r1969925177
##########
modules/partition-replicator/src/integrationTest/java/org/apache/ignite/internal/partition/replicator/ItReplicaLifecycleTest.java:
##########
@@ -658,6 +662,47 @@ public void testScanCloseReplicaRequest() throws Exception {
assertDoesNotThrow(tx::commit);
}
+ @Test
+ public void testReplicaSafeTimeSyncRequest() throws Exception {
+ startCluster(2);
+ Node node = getNode(0);
+
+ placementDriver.setPrimary(node.clusterService.topologyService().localMember());
+
+ // Prepare a zone.
+ String zoneName = "test_zone";
+ createZone(node, zoneName, 1, 2);
+ int zoneId = assertDoesNotThrow(() -> DistributionZonesTestUtil.getZoneIdStrict(
Review Comment:
What's `assertDoesNotThrow` for?
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22620 Implement ReplicaSafeTimeSyncRequest processing for zone replica [ignite-3]
Posted by "sanpwc (via GitHub)" <gi...@apache.org>.
sanpwc commented on code in PR #5258:
URL: https://github.com/apache/ignite-3/pull/5258#discussion_r1965825416
##########
modules/partition-replicator/src/integrationTest/java/org/apache/ignite/internal/partition/replicator/ItReplicaLifecycleTest.java:
##########
@@ -656,6 +660,41 @@ public void testScanCloseReplicaRequest() throws Exception {
assertDoesNotThrow(tx::commit);
}
+ @Test
+ public void testReplicaSafeTimeSyncRequest() throws Exception {
+ // Prepare a single node cluster.
+ startCluster(2);
+ Node node0 = getNode(0);
+ List<Set<Assignment>> assignments = PartitionDistributionUtils.calculateAssignments(
+ cluster.stream().map(n -> n.name).collect(toList()), 1, 1);
+
+ List<TokenizedAssignments> tokenizedAssignments = assignments.stream()
+ .map(a -> new TokenizedAssignmentsImpl(a, Integer.MIN_VALUE))
+ .collect(toList());
+
+ placementDriver.setPrimary(node0.clusterService.topologyService().localMember());
+ placementDriver.setAssignments(tokenizedAssignments);
+
+ // Prepare a zone.
+ String zoneName = "test_zone";
+ createZone(node0, zoneName, 1, 2);
+ int zoneId = DistributionZonesTestUtil.getZoneId(node0.catalogManager, zoneName, node0.hybridClock.nowLong());
+ int partId = 0;
+
+ // Create a table to work with.
+ createTable(node0, zoneName, "test_table");
Review Comment:
Please adjust the test in a following way.
1. Create zone
2. ensure that safeTime was successfully adjusted. // It should be possible to adjust timestamp even if there's no tables in zone.
3. Create table
4. await table creation and ensure that safeTime was successfully adjusted.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22620 Implement ReplicaSafeTimeSyncRequest processing for zone replica [ignite-3]
Posted by "JAkutenshi (via GitHub)" <gi...@apache.org>.
JAkutenshi commented on code in PR #5258:
URL: https://github.com/apache/ignite-3/pull/5258#discussion_r1981728618
##########
modules/partition-replicator/src/main/java/org/apache/ignite/internal/partition/replicator/raft/ZonePartitionRaftListener.java:
##########
@@ -268,13 +272,24 @@ private IgniteBiTuple<Serializable, Boolean> processCrossTableProcessorsCommand(
) {
IgniteBiTuple<Serializable, Boolean> result = new IgniteBiTuple<>(null, false);
- tableProcessors.values().forEach(processor -> {
- IgniteBiTuple<Serializable, Boolean> r = processor.processCommand(command, commandIndex, commandTerm, safeTimestamp);
- // Need to adjust the safe time if any of the table processors successfully handled the command.
- if (Boolean.TRUE.equals(r.get2())) {
- result.set2(Boolean.TRUE);
+ if (tableProcessors.isEmpty()) {
+ synchronized (tableProcessorsStateLock) {
+ if (lastAppliedIndex < commandIndex) {
+ lastAppliedIndex = commandIndex;
Review Comment:
Fair, changed
##########
modules/partition-replicator/src/main/java/org/apache/ignite/internal/partition/replicator/raft/ZonePartitionRaftListener.java:
##########
@@ -268,13 +272,24 @@ private IgniteBiTuple<Serializable, Boolean> processCrossTableProcessorsCommand(
) {
IgniteBiTuple<Serializable, Boolean> result = new IgniteBiTuple<>(null, false);
- tableProcessors.values().forEach(processor -> {
- IgniteBiTuple<Serializable, Boolean> r = processor.processCommand(command, commandIndex, commandTerm, safeTimestamp);
- // Need to adjust the safe time if any of the table processors successfully handled the command.
- if (Boolean.TRUE.equals(r.get2())) {
- result.set2(Boolean.TRUE);
+ if (tableProcessors.isEmpty()) {
+ synchronized (tableProcessorsStateLock) {
+ if (lastAppliedIndex < commandIndex) {
+ lastAppliedIndex = commandIndex;
+ lastAppliedTerm = commandTerm;
+
+ result.set2(Boolean.TRUE);
Review Comment:
Done
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22620 Implement ReplicaSafeTimeSyncRequest processing for zone replica [ignite-3]
Posted by "JAkutenshi (via GitHub)" <gi...@apache.org>.
JAkutenshi commented on code in PR #5258:
URL: https://github.com/apache/ignite-3/pull/5258#discussion_r1978988089
##########
modules/partition-replicator/src/integrationTest/java/org/apache/ignite/internal/partition/replicator/ItReplicaLifecycleTest.java:
##########
@@ -656,6 +660,41 @@ public void testScanCloseReplicaRequest() throws Exception {
assertDoesNotThrow(tx::commit);
}
+ @Test
+ public void testReplicaSafeTimeSyncRequest() throws Exception {
+ // Prepare a single node cluster.
+ startCluster(2);
+ Node node0 = getNode(0);
+ List<Set<Assignment>> assignments = PartitionDistributionUtils.calculateAssignments(
+ cluster.stream().map(n -> n.name).collect(toList()), 1, 1);
+
+ List<TokenizedAssignments> tokenizedAssignments = assignments.stream()
+ .map(a -> new TokenizedAssignmentsImpl(a, Integer.MIN_VALUE))
+ .collect(toList());
+
+ placementDriver.setPrimary(node0.clusterService.topologyService().localMember());
+ placementDriver.setAssignments(tokenizedAssignments);
+
+ // Prepare a zone.
+ String zoneName = "test_zone";
+ createZone(node0, zoneName, 1, 2);
+ int zoneId = DistributionZonesTestUtil.getZoneId(node0.catalogManager, zoneName, node0.hybridClock.nowLong());
+ int partId = 0;
+
+ // Create a table to work with.
+ createTable(node0, zoneName, "test_table");
+
+ HybridTimestamp node0safeTimeBefore = node0.currentSafeTimeForZonePartition(zoneId, partId);
Review Comment:
Added a check `waitForZoneReplicaStartedOnNode` that explicitly waits for a zone replica is started
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22620 Implement ReplicaSafeTimeSyncRequest processing for zone replica [ignite-3]
Posted by "sashapolo (via GitHub)" <gi...@apache.org>.
sashapolo commented on code in PR #5258:
URL: https://github.com/apache/ignite-3/pull/5258#discussion_r1969915500
##########
modules/partition-replicator/src/main/java/org/apache/ignite/internal/partition/replicator/raft/ZonePartitionRaftListener.java:
##########
@@ -239,7 +243,9 @@ private IgniteBiTuple<Serializable, Boolean> processCrossTableProcessorsCommand(
}
});
- return result;
+ return tableProcessors.isEmpty()
Review Comment:
It makes more sense to check this condition at the beginning of this method
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22620 Implement ReplicaSafeTimeSyncRequest processing for zone replica [ignite-3]
Posted by "sashapolo (via GitHub)" <gi...@apache.org>.
sashapolo commented on code in PR #5258:
URL: https://github.com/apache/ignite-3/pull/5258#discussion_r1981509214
##########
modules/partition-replicator/src/main/java/org/apache/ignite/internal/partition/replicator/raft/ZonePartitionRaftListener.java:
##########
@@ -268,13 +272,24 @@ private IgniteBiTuple<Serializable, Boolean> processCrossTableProcessorsCommand(
) {
IgniteBiTuple<Serializable, Boolean> result = new IgniteBiTuple<>(null, false);
- tableProcessors.values().forEach(processor -> {
- IgniteBiTuple<Serializable, Boolean> r = processor.processCommand(command, commandIndex, commandTerm, safeTimestamp);
- // Need to adjust the safe time if any of the table processors successfully handled the command.
- if (Boolean.TRUE.equals(r.get2())) {
- result.set2(Boolean.TRUE);
+ if (tableProcessors.isEmpty()) {
+ synchronized (tableProcessorsStateLock) {
+ if (lastAppliedIndex < commandIndex) {
+ lastAppliedIndex = commandIndex;
Review Comment:
you don't need to update the index and the term here, it will be done automatically by the enclosing method.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22620 Implement ReplicaSafeTimeSyncRequest processing for zone replica [ignite-3]
Posted by "JAkutenshi (via GitHub)" <gi...@apache.org>.
JAkutenshi commented on code in PR #5258:
URL: https://github.com/apache/ignite-3/pull/5258#discussion_r1981888513
##########
modules/partition-replicator/src/main/java/org/apache/ignite/internal/partition/replicator/raft/ZonePartitionRaftListener.java:
##########
@@ -266,9 +270,16 @@ private IgniteBiTuple<Serializable, Boolean> processCrossTableProcessorsCommand(
long commandTerm,
@Nullable HybridTimestamp safeTimestamp
) {
+ if (tableProcessors.isEmpty()) {
+ synchronized (tableProcessorsStateLock) {
+ return new IgniteBiTuple<>(null, lastAppliedIndex < commandIndex);
+ }
+ }
+
IgniteBiTuple<Serializable, Boolean> result = new IgniteBiTuple<>(null, false);
tableProcessors.values().forEach(processor -> {
+
Review Comment:
Oops
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22620 Implement ReplicaSafeTimeSyncRequest processing for zone replica [ignite-3]
Posted by "sashapolo (via GitHub)" <gi...@apache.org>.
sashapolo commented on code in PR #5258:
URL: https://github.com/apache/ignite-3/pull/5258#discussion_r1981510152
##########
modules/partition-replicator/src/integrationTest/java/org/apache/ignite/internal/partition/replicator/ItReplicaLifecycleTest.java:
##########
@@ -636,6 +639,54 @@ public void testScanCloseReplicaRequest() throws Exception {
assertDoesNotThrow(tx::commit);
}
+ @Test
+ public void testReplicaSafeTimeSyncRequest() throws Exception {
+ startCluster(2);
+ Node node = getNode(0);
+
+ // Prepare a zone.
+ String zoneName = "test_zone";
+ createZone(node, zoneName, 1, 2);
+ int zoneId = DistributionZonesTestUtil.getZoneId(node.catalogManager, zoneName, node.hybridClock.nowLong());
+ int partId = 0;
+
+ // Wait for sure that zone replication group was created on both nodes.
+ waitForZoneReplicaStartedOnNode(node, zoneId, partId);
+ waitForZoneReplicaStartedOnNode(getNode(1), zoneId, partId);
Review Comment:
It does look more verbose, but you had to introduce a separate method while an existing method would have solved this problem.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22620 Implement ReplicaSafeTimeSyncRequest processing for zone replica [ignite-3]
Posted by "JAkutenshi (via GitHub)" <gi...@apache.org>.
JAkutenshi commented on code in PR #5258:
URL: https://github.com/apache/ignite-3/pull/5258#discussion_r1981366270
##########
modules/partition-replicator/src/integrationTest/java/org/apache/ignite/internal/partition/replicator/ItReplicaLifecycleTest.java:
##########
@@ -636,6 +639,54 @@ public void testScanCloseReplicaRequest() throws Exception {
assertDoesNotThrow(tx::commit);
}
+ @Test
+ public void testReplicaSafeTimeSyncRequest() throws Exception {
+ startCluster(2);
+ Node node = getNode(0);
+
+ // Prepare a zone.
+ String zoneName = "test_zone";
+ createZone(node, zoneName, 1, 2);
+ int zoneId = DistributionZonesTestUtil.getZoneId(node.catalogManager, zoneName, node.hybridClock.nowLong());
+ int partId = 0;
+
+ // Wait for sure that zone replication group was created on both nodes.
+ waitForZoneReplicaStartedOnNode(node, zoneId, partId);
+ waitForZoneReplicaStartedOnNode(getNode(1), zoneId, partId);
Review Comment:
Not sure, but used method looks more verbose and precise for the situation, isn't?
##########
modules/partition-replicator/src/integrationTest/java/org/apache/ignite/internal/partition/replicator/ItReplicaLifecycleTest.java:
##########
@@ -636,6 +639,54 @@ public void testScanCloseReplicaRequest() throws Exception {
assertDoesNotThrow(tx::commit);
}
+ @Test
+ public void testReplicaSafeTimeSyncRequest() throws Exception {
+ startCluster(2);
+ Node node = getNode(0);
+
+ // Prepare a zone.
+ String zoneName = "test_zone";
+ createZone(node, zoneName, 1, 2);
+ int zoneId = DistributionZonesTestUtil.getZoneId(node.catalogManager, zoneName, node.hybridClock.nowLong());
+ int partId = 0;
+
+ // Wait for sure that zone replication group was created on both nodes.
+ waitForZoneReplicaStartedOnNode(node, zoneId, partId);
+ waitForZoneReplicaStartedOnNode(getNode(1), zoneId, partId);
+
+ // Check that time was adjusted even without table.
+ checkSafeTimeWasAdjustedForZoneGroup(zoneId, partId);
+
+ // Create a table to work with.
+ String tableName = "test_table";
+ createTable(node, zoneName, tableName);
+ assertDoesNotThrow(() -> TableTestUtils.getTableIdStrict(node.catalogManager, tableName, node.hybridClock.nowLong()));
+
+ // Check that time was adjusted table.
+ checkSafeTimeWasAdjustedForZoneGroup(zoneId, partId);
+ }
+
+ private static void waitForZoneReplicaStartedOnNode(Node node, int zoneId, int partId) throws InterruptedException {
+ ZonePartitionId zoneReplicationId = new ZonePartitionId(zoneId, partId);
+
+ waitForCondition(() -> node.replicaManager.replica(zoneReplicationId) != null, AWAIT_TIMEOUT_MILLIS);
Review Comment:
Fixed, thank you!
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22620 Implement ReplicaSafeTimeSyncRequest processing for zone replica [ignite-3]
Posted by "JAkutenshi (via GitHub)" <gi...@apache.org>.
JAkutenshi commented on code in PR #5258:
URL: https://github.com/apache/ignite-3/pull/5258#discussion_r1981727883
##########
modules/partition-replicator/src/integrationTest/java/org/apache/ignite/internal/partition/replicator/ItReplicaLifecycleTest.java:
##########
@@ -636,6 +639,54 @@ public void testScanCloseReplicaRequest() throws Exception {
assertDoesNotThrow(tx::commit);
}
+ @Test
+ public void testReplicaSafeTimeSyncRequest() throws Exception {
+ startCluster(2);
+ Node node = getNode(0);
+
+ // Prepare a zone.
+ String zoneName = "test_zone";
+ createZone(node, zoneName, 1, 2);
+ int zoneId = DistributionZonesTestUtil.getZoneId(node.catalogManager, zoneName, node.hybridClock.nowLong());
+ int partId = 0;
+
+ // Wait for sure that zone replication group was created on both nodes.
+ waitForZoneReplicaStartedOnNode(node, zoneId, partId);
+ waitForZoneReplicaStartedOnNode(getNode(1), zoneId, partId);
Review Comment:
Changed
##########
modules/partition-replicator/src/integrationTest/java/org/apache/ignite/internal/partition/replicator/ItReplicaLifecycleTest.java:
##########
@@ -636,6 +639,54 @@ public void testScanCloseReplicaRequest() throws Exception {
assertDoesNotThrow(tx::commit);
}
+ @Test
+ public void testReplicaSafeTimeSyncRequest() throws Exception {
+ startCluster(2);
+ Node node = getNode(0);
+
+ // Prepare a zone.
+ String zoneName = "test_zone";
+ createZone(node, zoneName, 1, 2);
+ int zoneId = DistributionZonesTestUtil.getZoneId(node.catalogManager, zoneName, node.hybridClock.nowLong());
+ int partId = 0;
+
+ // Wait for sure that zone replication group was created on both nodes.
+ waitForZoneReplicaStartedOnNode(node, zoneId, partId);
+ waitForZoneReplicaStartedOnNode(getNode(1), zoneId, partId);
+
+ // Check that time was adjusted even without table.
+ checkSafeTimeWasAdjustedForZoneGroup(zoneId, partId);
+
+ // Create a table to work with.
+ String tableName = "test_table";
+ createTable(node, zoneName, tableName);
+ assertDoesNotThrow(() -> TableTestUtils.getTableIdStrict(node.catalogManager, tableName, node.hybridClock.nowLong()));
Review Comment:
Oh, didn't parse what actual line is problematic. Fair, removed.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22620 Implement ReplicaSafeTimeSyncRequest processing for zone replica [ignite-3]
Posted by "JAkutenshi (via GitHub)" <gi...@apache.org>.
JAkutenshi commented on code in PR #5258:
URL: https://github.com/apache/ignite-3/pull/5258#discussion_r1969720745
##########
modules/partition-replicator/src/integrationTest/java/org/apache/ignite/internal/partition/replicator/ItReplicaLifecycleTest.java:
##########
@@ -656,6 +660,41 @@ public void testScanCloseReplicaRequest() throws Exception {
assertDoesNotThrow(tx::commit);
}
+ @Test
+ public void testReplicaSafeTimeSyncRequest() throws Exception {
+ // Prepare a single node cluster.
Review Comment:
Fixed
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22620 Implement ReplicaSafeTimeSyncRequest processing for zone replica [ignite-3]
Posted by "JAkutenshi (via GitHub)" <gi...@apache.org>.
JAkutenshi commented on code in PR #5258:
URL: https://github.com/apache/ignite-3/pull/5258#discussion_r1981367151
##########
modules/partition-replicator/src/integrationTest/java/org/apache/ignite/internal/partition/replicator/ItReplicaLifecycleTest.java:
##########
@@ -636,6 +639,54 @@ public void testScanCloseReplicaRequest() throws Exception {
assertDoesNotThrow(tx::commit);
}
+ @Test
+ public void testReplicaSafeTimeSyncRequest() throws Exception {
+ startCluster(2);
+ Node node = getNode(0);
+
+ // Prepare a zone.
+ String zoneName = "test_zone";
+ createZone(node, zoneName, 1, 2);
+ int zoneId = DistributionZonesTestUtil.getZoneId(node.catalogManager, zoneName, node.hybridClock.nowLong());
+ int partId = 0;
+
+ // Wait for sure that zone replication group was created on both nodes.
+ waitForZoneReplicaStartedOnNode(node, zoneId, partId);
+ waitForZoneReplicaStartedOnNode(getNode(1), zoneId, partId);
+
+ // Check that time was adjusted even without table.
+ checkSafeTimeWasAdjustedForZoneGroup(zoneId, partId);
+
+ // Create a table to work with.
+ String tableName = "test_table";
+ createTable(node, zoneName, tableName);
+ assertDoesNotThrow(() -> TableTestUtils.getTableIdStrict(node.catalogManager, tableName, node.hybridClock.nowLong()));
+
+ // Check that time was adjusted table.
+ checkSafeTimeWasAdjustedForZoneGroup(zoneId, partId);
+ }
+
+ private static void waitForZoneReplicaStartedOnNode(Node node, int zoneId, int partId) throws InterruptedException {
+ ZonePartitionId zoneReplicationId = new ZonePartitionId(zoneId, partId);
+
+ waitForCondition(() -> node.replicaManager.replica(zoneReplicationId) != null, AWAIT_TIMEOUT_MILLIS);
+ assertThat(node.replicaManager.replica(zoneReplicationId), willCompleteSuccessfully());
+ }
+
+ private void checkSafeTimeWasAdjustedForZoneGroup(int zoneId, int partId) throws InterruptedException {
Review Comment:
I won't use a lot of node(1), but I agree that your proposal is cleaner, changed.
##########
modules/partition-replicator/src/integrationTest/java/org/apache/ignite/internal/partition/replicator/ItReplicaLifecycleTest.java:
##########
@@ -636,6 +639,54 @@ public void testScanCloseReplicaRequest() throws Exception {
assertDoesNotThrow(tx::commit);
}
+ @Test
+ public void testReplicaSafeTimeSyncRequest() throws Exception {
+ startCluster(2);
+ Node node = getNode(0);
+
+ // Prepare a zone.
+ String zoneName = "test_zone";
+ createZone(node, zoneName, 1, 2);
+ int zoneId = DistributionZonesTestUtil.getZoneId(node.catalogManager, zoneName, node.hybridClock.nowLong());
+ int partId = 0;
+
+ // Wait for sure that zone replication group was created on both nodes.
+ waitForZoneReplicaStartedOnNode(node, zoneId, partId);
+ waitForZoneReplicaStartedOnNode(getNode(1), zoneId, partId);
+
+ // Check that time was adjusted even without table.
+ checkSafeTimeWasAdjustedForZoneGroup(zoneId, partId);
+
+ // Create a table to work with.
+ String tableName = "test_table";
+ createTable(node, zoneName, tableName);
+ assertDoesNotThrow(() -> TableTestUtils.getTableIdStrict(node.catalogManager, tableName, node.hybridClock.nowLong()));
Review Comment:
Fixed
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22620 Implement ReplicaSafeTimeSyncRequest processing for zone replica [ignite-3]
Posted by "JAkutenshi (via GitHub)" <gi...@apache.org>.
JAkutenshi commented on code in PR #5258:
URL: https://github.com/apache/ignite-3/pull/5258#discussion_r1981367289
##########
modules/partition-replicator/src/integrationTest/java/org/apache/ignite/internal/partition/replicator/ItReplicaLifecycleTest.java:
##########
@@ -636,6 +639,54 @@ public void testScanCloseReplicaRequest() throws Exception {
assertDoesNotThrow(tx::commit);
}
+ @Test
+ public void testReplicaSafeTimeSyncRequest() throws Exception {
+ startCluster(2);
+ Node node = getNode(0);
+
+ // Prepare a zone.
+ String zoneName = "test_zone";
+ createZone(node, zoneName, 1, 2);
+ int zoneId = DistributionZonesTestUtil.getZoneId(node.catalogManager, zoneName, node.hybridClock.nowLong());
+ int partId = 0;
+
+ // Wait for sure that zone replication group was created on both nodes.
+ waitForZoneReplicaStartedOnNode(node, zoneId, partId);
+ waitForZoneReplicaStartedOnNode(getNode(1), zoneId, partId);
+
+ // Check that time was adjusted even without table.
+ checkSafeTimeWasAdjustedForZoneGroup(zoneId, partId);
+
+ // Create a table to work with.
+ String tableName = "test_table";
+ createTable(node, zoneName, tableName);
+ assertDoesNotThrow(() -> TableTestUtils.getTableIdStrict(node.catalogManager, tableName, node.hybridClock.nowLong()));
Review Comment:
Fixed
##########
modules/partition-replicator/src/integrationTest/java/org/apache/ignite/internal/partition/replicator/ItReplicaLifecycleTest.java:
##########
@@ -636,6 +639,54 @@ public void testScanCloseReplicaRequest() throws Exception {
assertDoesNotThrow(tx::commit);
}
+ @Test
+ public void testReplicaSafeTimeSyncRequest() throws Exception {
+ startCluster(2);
+ Node node = getNode(0);
+
+ // Prepare a zone.
+ String zoneName = "test_zone";
+ createZone(node, zoneName, 1, 2);
+ int zoneId = DistributionZonesTestUtil.getZoneId(node.catalogManager, zoneName, node.hybridClock.nowLong());
+ int partId = 0;
+
+ // Wait for sure that zone replication group was created on both nodes.
+ waitForZoneReplicaStartedOnNode(node, zoneId, partId);
+ waitForZoneReplicaStartedOnNode(getNode(1), zoneId, partId);
+
+ // Check that time was adjusted even without table.
+ checkSafeTimeWasAdjustedForZoneGroup(zoneId, partId);
+
+ // Create a table to work with.
+ String tableName = "test_table";
+ createTable(node, zoneName, tableName);
+ assertDoesNotThrow(() -> TableTestUtils.getTableIdStrict(node.catalogManager, tableName, node.hybridClock.nowLong()));
+
+ // Check that time was adjusted table.
+ checkSafeTimeWasAdjustedForZoneGroup(zoneId, partId);
+ }
+
+ private static void waitForZoneReplicaStartedOnNode(Node node, int zoneId, int partId) throws InterruptedException {
+ ZonePartitionId zoneReplicationId = new ZonePartitionId(zoneId, partId);
+
+ waitForCondition(() -> node.replicaManager.replica(zoneReplicationId) != null, AWAIT_TIMEOUT_MILLIS);
+ assertThat(node.replicaManager.replica(zoneReplicationId), willCompleteSuccessfully());
+ }
+
+ private void checkSafeTimeWasAdjustedForZoneGroup(int zoneId, int partId) throws InterruptedException {
+ Node node0 = getNode(0);
+ Node node1 = getNode(1);
+
+ HybridTimestamp node0safeTimeBefore = node0.currentSafeTimeForZonePartition(zoneId, partId);
+ HybridTimestamp node1safeTimeBefore = node1.currentSafeTimeForZonePartition(zoneId, partId);
+
+ waitForCondition(
Review Comment:
Fixed
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22620 Implement ReplicaSafeTimeSyncRequest processing for zone replica [ignite-3]
Posted by "JAkutenshi (via GitHub)" <gi...@apache.org>.
JAkutenshi commented on code in PR #5258:
URL: https://github.com/apache/ignite-3/pull/5258#discussion_r1981369590
##########
modules/partition-replicator/src/integrationTest/java/org/apache/ignite/internal/partition/replicator/ItReplicaLifecycleTest.java:
##########
@@ -636,6 +639,54 @@ public void testScanCloseReplicaRequest() throws Exception {
assertDoesNotThrow(tx::commit);
}
+ @Test
+ public void testReplicaSafeTimeSyncRequest() throws Exception {
+ startCluster(2);
+ Node node = getNode(0);
+
+ // Prepare a zone.
+ String zoneName = "test_zone";
+ createZone(node, zoneName, 1, 2);
+ int zoneId = DistributionZonesTestUtil.getZoneId(node.catalogManager, zoneName, node.hybridClock.nowLong());
+ int partId = 0;
+
+ // Wait for sure that zone replication group was created on both nodes.
+ waitForZoneReplicaStartedOnNode(node, zoneId, partId);
+ waitForZoneReplicaStartedOnNode(getNode(1), zoneId, partId);
+
+ // Check that time was adjusted even without table.
Review Comment:
Applied, thanks
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22620 Implement ReplicaSafeTimeSyncRequest processing for zone replica [ignite-3]
Posted by "sashapolo (via GitHub)" <gi...@apache.org>.
sashapolo commented on code in PR #5258:
URL: https://github.com/apache/ignite-3/pull/5258#discussion_r1981498219
##########
modules/partition-replicator/src/integrationTest/java/org/apache/ignite/internal/partition/replicator/ItReplicaLifecycleTest.java:
##########
@@ -636,6 +639,54 @@ public void testScanCloseReplicaRequest() throws Exception {
assertDoesNotThrow(tx::commit);
}
+ @Test
+ public void testReplicaSafeTimeSyncRequest() throws Exception {
+ startCluster(2);
+ Node node = getNode(0);
+
+ // Prepare a zone.
+ String zoneName = "test_zone";
+ createZone(node, zoneName, 1, 2);
+ int zoneId = DistributionZonesTestUtil.getZoneId(node.catalogManager, zoneName, node.hybridClock.nowLong());
+ int partId = 0;
+
+ // Wait for sure that zone replication group was created on both nodes.
+ waitForZoneReplicaStartedOnNode(node, zoneId, partId);
+ waitForZoneReplicaStartedOnNode(getNode(1), zoneId, partId);
+
+ // Check that time was adjusted even without table.
+ checkSafeTimeWasAdjustedForZoneGroup(zoneId, partId);
+
+ // Create a table to work with.
+ String tableName = "test_table";
+ createTable(node, zoneName, tableName);
+ assertDoesNotThrow(() -> TableTestUtils.getTableIdStrict(node.catalogManager, tableName, node.hybridClock.nowLong()));
Review Comment:
yes, that's why you call `createTable` above, why do you need this check as well?
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-22620 Implement ReplicaSafeTimeSyncRequest processing for zone replica [ignite-3]
Posted by "sashapolo (via GitHub)" <gi...@apache.org>.
sashapolo merged PR #5258:
URL: https://github.com/apache/ignite-3/pull/5258
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org