rpuch commented on code in PR #3220:
URL: https://github.com/apache/ignite-3/pull/3220#discussion_r1500669106
##########
modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/topology/api/LogicalTopologyService.java:
##########
@@ -62,6 +62,11 @@ public interface LogicalTopologyService {
*/
CompletableFuture<LogicalTopologySnapshot> logicalTopologyOnLeader();
+ /**
+ * Retrieves the current logical topology snapshot stored in the local
storage.
+ */
+ LogicalTopologySnapshot getLogicalTopology();
Review Comment:
Does it make sense to reflect it in the name of the method that the topology
is returned based on the local view of the node? Like `localLogicalTopology()`.
Also, `get` prefix does not match the style of `logicalTopologyOnLeader()`:
they should both have `get` prefix, or none of them should have it.
##########
modules/runner/src/main/java/org/apache/ignite/internal/app/IgniteImpl.java:
##########
@@ -990,6 +994,37 @@ public CompletableFuture<Ignite> start(Path configPath) {
return cmgMgr.onJoinReady();
}, startupExecutor)
+ .thenComposeAsync(ignored -> {
+ CompletableFuture<Void>
awaitSelfInLogicalTopologyFuture = new CompletableFuture<>();
+
+ LogicalTopologyEventListener awaitSelfListener = new
LogicalTopologyEventListener() {
+ @Override
+ public void onNodeJoined(LogicalNode joinedNode,
LogicalTopologySnapshot newTopology) {
+ if
(newTopology.nodes().stream().map(LogicalNode::id).collect(Collectors.toSet()).contains(id()))
{
Review Comment:
If `toSet` is imported statically, this will read nicely in plain English:
'collect to set'
##########
modules/runner/src/main/java/org/apache/ignite/internal/app/IgniteImpl.java:
##########
@@ -990,6 +994,37 @@ public CompletableFuture<Ignite> start(Path configPath) {
return cmgMgr.onJoinReady();
}, startupExecutor)
+ .thenComposeAsync(ignored -> {
+ CompletableFuture<Void>
awaitSelfInLogicalTopologyFuture = new CompletableFuture<>();
+
+ LogicalTopologyEventListener awaitSelfListener = new
LogicalTopologyEventListener() {
+ @Override
+ public void onNodeJoined(LogicalNode joinedNode,
LogicalTopologySnapshot newTopology) {
+ if
(newTopology.nodes().stream().map(LogicalNode::id).collect(Collectors.toSet()).contains(id()))
{
+
awaitSelfInLogicalTopologyFuture.complete(null);
+
logicalTopologyService.removeEventListener(this);
+ }
+ }
+
+ @Override
+ public void onTopologyLeap(LogicalTopologySnapshot
newTopology) {
+ if
(newTopology.nodes().stream().map(LogicalNode::id).collect(Collectors.toSet()).contains(id()))
{
Review Comment:
It seems like there are 3 duplicates of this code, it it possible to extract
them to methods?
##########
modules/runner/src/main/java/org/apache/ignite/internal/app/IgniteImpl.java:
##########
@@ -990,6 +994,37 @@ public CompletableFuture<Ignite> start(Path configPath) {
return cmgMgr.onJoinReady();
}, startupExecutor)
+ .thenComposeAsync(ignored -> {
+ CompletableFuture<Void>
awaitSelfInLogicalTopologyFuture = new CompletableFuture<>();
Review Comment:
Is it possible to extract this as a method? The method in which this code is
defined is already huge.
--
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]