JAkutenshi commented on code in PR #5955:
URL: https://github.com/apache/ignite-3/pull/5955#discussion_r2120981378
##########
modules/partition-replicator/src/integrationTest/java/org/apache/ignite/internal/partition/replicator/fixtures/Node.java:
##########
@@ -997,4 +1029,13 @@ public ReplicaMeta getPrimaryReplica(int zoneId) throws
InterruptedException {
assertThat(primaryReplicaFuture, willCompleteSuccessfully());
return primaryReplicaFuture.join();
}
+
+ @Contract("null -> null")
Review Comment:
Just to say I didn't see this annotation previous, interesting thing
##########
modules/partition-replicator/src/integrationTest/java/org/apache/ignite/internal/partition/replicator/fixtures/Node.java:
##########
@@ -763,19 +783,31 @@ public CompletableFuture<Boolean> invoke(
@Override
protected MvTableStorage createTableStorage(CatalogTableDescriptor
tableDescriptor, CatalogZoneDescriptor zoneDescriptor) {
- MvTableStorage storage =
super.createTableStorage(tableDescriptor, zoneDescriptor);
+ MvTableStorage storage =
createSpy(super.createTableStorage(tableDescriptor, zoneDescriptor));
Review Comment:
Why we need mocks there instead of real storages?
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -1092,6 +1093,10 @@ private CompletableFuture<Boolean>
onTablePrimaryReplicaExpired(PrimaryReplicaEv
return falseCompletedFuture();
}
+ private boolean thisNodeHoldsLease(@Nullable UUID leaseholderId) {
Review Comment:
`doesLocalNodeHoldLease`?
1. Does brings question semantics for boolean returned method name
2. Local node more conventional within the class than "this".
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -979,106 +982,104 @@ private void
preparePartitionResourcesAndLoadToZoneReplica(TableImpl table, Zone
var tablePartitionId = new TablePartitionId(tableId, partId);
- inBusyLock(busyLock, () -> {
Review Comment:
1. Why to remove busy lock?
2. Why the method'sw name is `*Busy` then?
--
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-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
