sashapolo commented on code in PR #5300:
URL: https://github.com/apache/ignite-3/pull/5300#discussion_r1976585671
##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/TableManager.java:
##########
@@ -2814,7 +2835,11 @@ private CompletableFuture<Void>
stopPartition(TablePartitionId tablePartitionId,
CompletableFuture<Boolean> stopReplicaFuture;
try {
- stopReplicaFuture = replicaMgr.stopReplica(tablePartitionId);
+ // In case of colocation there shouldn't be any table replica and
thus it shouldn't be stopped. Moreover the excessive replica
Review Comment:
> Huh, yes, I agree, but why I left it as is: because it was such before me
and there might be a resource release protection (who knows why replica wasn't
stopped, but there still shouldn't be the raft node).
I don't like this approach, please find out why it was ignored, because it
might be a bug. Just because someone wrote it before doesn't mean it's correct.
I agree that it can be fixed in a different PR, if you want.
> We have a raft-node per zone partition, that starts in
PartitionReplicaLifecycleManager#createZonePartitionReplicationNode. We
shouldn't have the node with table and I want to play it safe there.
Yes, but here we are stopping a table-partition replica that shouldn't
exist, therefore stopping of the Raft node is a no-op. Therefore I don't
understand the problem described in the 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-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
