IGNITE-28731 Create cluster auto activation plugin - #355
Conversation
e1ba8c9 to
426bd4b
Compare
0d6b493 to
115b23d
Compare
|
|
||
| /** {@inheritDoc} */ | ||
| @Override public boolean apply(Collection<ClusterNode> nodes) { | ||
| Set<String> missingNodes = new LinkedHashSet<>(requiredNodes); |
There was a problem hiding this comment.
| Set<String> missingNodes = new LinkedHashSet<>(requiredNodes); | |
| Set<String> missingNodes = new HashSet<>(requiredNodes); |
| if (condition.apply(cluster.nodes())) { | ||
| if (logger.isInfoEnabled()) | ||
| logger.info("Auto activation plugin set cluster state ACTIVE - activation condition meet"); | ||
|
|
||
| cluster.state(ClusterState.ACTIVE); | ||
| } | ||
| else { | ||
| if (logger.isInfoEnabled()) | ||
| logger.info("Auto activation skipped - activation condition not meet"); | ||
| } | ||
| } |
There was a problem hiding this comment.
| if (condition.apply(cluster.nodes())) { | |
| if (logger.isInfoEnabled()) | |
| logger.info("Auto activation plugin set cluster state ACTIVE - activation condition meet"); | |
| cluster.state(ClusterState.ACTIVE); | |
| } | |
| else { | |
| if (logger.isInfoEnabled()) | |
| logger.info("Auto activation skipped - activation condition not meet"); | |
| } | |
| } | |
| if (condition.apply(cluster.nodes())) { | |
| if (logger.isInfoEnabled()) | |
| logger.info("Auto activation plugin set cluster state ACTIVE - activation condition meet"); | |
| cluster.state(ClusterState.ACTIVE); | |
| return; | |
| } | |
| if (logger.isInfoEnabled()) | |
| logger.info("Auto activation skipped - activation condition not meet"); | |
| } |
There was a problem hiding this comment.
Usually it's better to use extra return instead of else
|
|
||
| /** {@inheritDoc} */ | ||
| @Override public String copyright() { | ||
| return ""; |
There was a problem hiding this comment.
| return ""; | |
| return "Apache Software Foundation"; |
There was a problem hiding this comment.
Same as in org.apache.ignite.cdc.conflictresolve.CacheVersionConflictResolverPluginProvider#copyright
|
|
||
| /** {@inheritDoc} */ | ||
| @Override public boolean apply(Collection<ClusterNode> nodes) { | ||
| Set<String> missingNodes = new LinkedHashSet<>(requiredValues); |
There was a problem hiding this comment.
| Set<String> missingNodes = new LinkedHashSet<>(requiredValues); | |
| Set<String> missingNodes = new HashSet<>(requiredValues); |
There was a problem hiding this comment.
Better to use HashSet when we do not need insertion order for iteration
| <!-- | ||
| POM file. | ||
| --> |
There was a problem hiding this comment.
| <!-- | |
| POM file. | |
| --> |
There was a problem hiding this comment.
Delete to fix Ignoring multiple XML header comment! warning for command mvn clean install -pl :ignite-auto-activation-ext -am -Pcheckstyle -DskipTests
| return; | ||
| } | ||
|
|
||
| if (condition.apply(cluster.nodes())) { |
There was a problem hiding this comment.
Can we use cluster.forServers().nodes() here? Then we do not need client checks in both predicate classes.
Or replace IgniteException("Auto-activation-plugin supports on with return false
There was a problem hiding this comment.
Good suggestion! I've applied the cluster.forServers().nodes()
| import org.apache.ignite.plugin.PluginValidationException; | ||
|
|
||
| /** | ||
| * Activate cluster when specified condition meet |
There was a problem hiding this comment.
| * Activate cluster when specified condition meet | |
| * Activate cluster when specified condition meet. |
| import static org.apache.ignite.testframework.GridTestUtils.assertThrows; | ||
|
|
||
| /** | ||
| * {@link AutoActivationPluginProvider} test |
There was a problem hiding this comment.
| * {@link AutoActivationPluginProvider} test | |
| * Tests {@link AutoActivationPluginProvider}. |
| - Cluster baseline is not empty | ||
| - `condition` contains any client node | ||
|
|
||
| Depending on how you use Ignite, you can an extension using one of the following methods: |
There was a problem hiding this comment.
you can an extension using one
looks like a verb is missing
There was a problem hiding this comment.
Certainly missed it. Fixed.
| switch (igniteInstanceName) { | ||
| case NODE_0: | ||
| igniteConfiguration.setConsistentId(NODE_0); | ||
| break; | ||
|
|
||
| case NODE_1: | ||
| igniteConfiguration.setConsistentId(NODE_1); | ||
| break; | ||
|
|
||
| case NODE_2: | ||
| igniteConfiguration.setConsistentId(NODE_2); | ||
| break; | ||
|
|
||
| default: throw new IllegalArgumentException("Unknown node: " + igniteInstanceName); | ||
| } |
There was a problem hiding this comment.
Looks like switch is overhead here and we can simplify to smth like:
return super.getConfiguration(igniteInstanceName)
.setConsistentId(igniteInstanceName)
.setClusterStateOnStart(INACTIVE)
.setGridLogger(listeningLog);There was a problem hiding this comment.
Oh, this is much better! Done
chesnokoff
left a comment
There was a problem hiding this comment.
Many tests repeat the same node startup sequence and cluster state checks. Can we extract common helper methods for starting configured nodes and checking the expected state? This would make the individual test scenarios shorter and easier to understand
| @Override public boolean apply(Collection<ClusterNode> nodes) { | ||
| Set<String> missingNodes = new HashSet<>(requiredNodes); | ||
|
|
||
| for (ClusterNode node : nodes) { | ||
| String nodeConsistentId = node.consistentId().toString(); | ||
|
|
||
| missingNodes.remove(nodeConsistentId); | ||
|
|
||
| if (missingNodes.isEmpty()) | ||
| break; | ||
| } | ||
|
|
||
| return missingNodes.isEmpty(); | ||
| } |
There was a problem hiding this comment.
| @Override public boolean apply(Collection<ClusterNode> nodes) { | |
| Set<String> missingNodes = new HashSet<>(requiredNodes); | |
| for (ClusterNode node : nodes) { | |
| String nodeConsistentId = node.consistentId().toString(); | |
| missingNodes.remove(nodeConsistentId); | |
| if (missingNodes.isEmpty()) | |
| break; | |
| } | |
| return missingNodes.isEmpty(); | |
| } | |
| @Override public boolean apply(Collection<ClusterNode> nodes) { | |
| Set<String> missingNodes = new HashSet<>(requiredNodes); | |
| for (ClusterNode node : nodes) { | |
| String nodeConsistentId = node.consistentId().toString(); | |
| missingNodes.remove(nodeConsistentId); | |
| if (missingNodes.isEmpty()) | |
| return true; | |
| } | |
| return false; | |
| } |
| @Override public boolean apply(Collection<ClusterNode> nodes) { | ||
| Set<String> missingNodes = new HashSet<>(requiredValues); | ||
|
|
||
| for (ClusterNode node : nodes) { | ||
| String attrVal = node.attribute(attrName); | ||
|
|
||
| missingNodes.remove(attrVal); | ||
|
|
||
| if (missingNodes.isEmpty()) | ||
| break; | ||
| } | ||
|
|
||
| return missingNodes.isEmpty(); | ||
| } |
There was a problem hiding this comment.
| @Override public boolean apply(Collection<ClusterNode> nodes) { | |
| Set<String> missingNodes = new HashSet<>(requiredValues); | |
| for (ClusterNode node : nodes) { | |
| String attrVal = node.attribute(attrName); | |
| missingNodes.remove(attrVal); | |
| if (missingNodes.isEmpty()) | |
| break; | |
| } | |
| return missingNodes.isEmpty(); | |
| } | |
| @Override public boolean apply(Collection<ClusterNode> nodes) { | |
| Set<String> missingNodes = new HashSet<>(requiredValues); | |
| for (ClusterNode node : nodes) { | |
| String attrVal = node.attribute(attrName); | |
| missingNodes.remove(attrVal); | |
| if (missingNodes.isEmpty()) | |
| return true; | |
| } | |
| return false; | |
| } |
|
|
||
| /** {@inheritDoc} */ | ||
| @Override public void onIgniteStart() { | ||
|
|
| } | ||
|
|
||
| /** @return IgniteConfiguration from XML. */ | ||
| private IgniteConfiguration getConfigurationFromXml(String xmlPath) throws Exception { |
There was a problem hiding this comment.
From IDEA: Exception 'java.lang.Exception' is never thrown in the method
| PluginProvider<?> autoActivationProvider = new AutoActivationPluginProvider( | ||
| new ActivateByConsistentID(Set.of(NODE_0, NODE_1)) | ||
| ); | ||
|
|
| PluginProvider<?> autoActivationProvider = new AutoActivationPluginProvider( | ||
| new ActivateByConsistentID(Set.of(NODE_2)) | ||
| ); | ||
|
|
| PluginProvider<?> autoActivationProvider = new AutoActivationPluginProvider( | ||
| new ActivateByConsistentID(Set.of(NODE_2)) | ||
| ); | ||
|
|
There was a problem hiding this comment.
same for other methods: remove empty lines as first lines in method bodies
| private CacheConfiguration getCacheConfiguration() { | ||
| return new CacheConfiguration<>() |
There was a problem hiding this comment.
| private CacheConfiguration getCacheConfiguration() { | |
| return new CacheConfiguration<>() | |
| private CacheConfiguration<String, Integer> getCacheConfiguration() { | |
| return new CacheConfiguration<String, Integer>() |
There was a problem hiding this comment.
let's avoid raw usage of type
No description provided.