You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2021/07/12 19:50:47 UTC
[GitHub] [ignite] Erixonich opened a new pull request #9248: IGNITE-14728 Change IGNITE_PDS_WAL_REBALANCE_THRESHOLD from System property to Distributed property.
Erixonich opened a new pull request #9248:
URL: https://github.com/apache/ignite/pull/9248
Thank you for submitting the pull request to the Apache Ignite.
In order to streamline the review of the contribution
we ask you to ensure the following steps have been taken:
### The Contribution Checklist
- [ ] There is a single JIRA ticket related to the pull request.
- [ ] The web-link to the pull request is attached to the JIRA ticket.
- [ ] The JIRA ticket has the _Patch Available_ state.
- [ ] The pull request body describes changes that have been made.
The description explains _WHAT_ and _WHY_ was made instead of _HOW_.
- [ ] The pull request title is treated as the final commit message.
The following pattern must be used: `IGNITE-XXXX Change summary` where `XXXX` - number of JIRA issue.
- [ ] A reviewer has been mentioned through the JIRA comments
(see [the Maintainers list](https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute#HowtoContribute-ReviewProcessandMaintainers))
- [ ] The pull request has been checked by the Teamcity Bot and
the `green visa` attached to the JIRA ticket (see [TC.Bot: Check PR](https://mtcga.gridgain.com/prs.html))
### Notes
- [How to Contribute](https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute)
- [Coding abbreviation rules](https://cwiki.apache.org/confluence/display/IGNITE/Abbreviation+Rules)
- [Coding Guidelines](https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines)
- [Apache Ignite Teamcity Bot](https://cwiki.apache.org/confluence/display/IGNITE/Apache+Ignite+Teamcity+Bot)
If you need any help, please email dev@ignite.apache.org or ask anу advice on http://asf.slack.com _#ignite_ channel.
--
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
[GitHub] [ignite] Erixonich commented on a change in pull request #9248: IGNITE-14728 Change IGNITE_PDS_WAL_REBALANCE_THRESHOLD from System property to Distributed property.
Posted by GitBox <gi...@apache.org>.
Erixonich commented on a change in pull request #9248:
URL: https://github.com/apache/ignite/pull/9248#discussion_r671168532
##########
File path: modules/control-utility/src/test/java/org/apache/ignite/util/GridCommandHandlerPropertiesTest.java
##########
@@ -197,4 +199,47 @@ public void testPropertyDefaultQueryTimeout() {
)
);
}
+
+ /**
+ * Check the set command for property 'history.rebalance.threshold'.
+ */
+ @Test
+ public void testPropertyWalRebalanceThreshold() {
+ assertDistributedPropertyEquals(HISTORICAL_REBALANCE_THRESHOLD_DMS_KEY, DFLT_PDS_WAL_REBALANCE_THRESHOLD, true);
+
+ int newVal = DFLT_PDS_WAL_REBALANCE_THRESHOLD * 2;
+
+ assertEquals(
+ EXIT_CODE_OK,
+ execute(
+ "--property", "set",
+ "--name", HISTORICAL_REBALANCE_THRESHOLD_DMS_KEY,
+ "--val", Integer.toString(newVal)
+ )
+ );
+
+ assertDistributedPropertyEquals(HISTORICAL_REBALANCE_THRESHOLD_DMS_KEY, newVal, true);
+ }
+
+ /**
+ * Validates that distributed property has specified value across all nodes.
+ *
+ * @param propName Distributed property name.
+ * @param expected Expected property value.
+ * @param onlyServerMode Ignore client nodes.
+ * @param <T> Property type.
+ */
+ private <T extends Serializable> void assertDistributedPropertyEquals(String propName, T expected, boolean onlyServerMode) {
+ for (Ignite ign : G.allGrids()) {
+ IgniteEx ignEx = (IgniteEx) ign;
+
+ if (onlyServerMode && ign.configuration().isClientMode())
+ continue;
+
+ DistributedChangeableProperty<Serializable> prop =
+ ignEx.context().distributedConfiguration().property(propName);
+
+ assertEquals(prop.get(), expected);
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
[GitHub] [ignite] sk0x50 commented on a change in pull request #9248: IGNITE-14728 Change IGNITE_PDS_WAL_REBALANCE_THRESHOLD from System property to Distributed property.
Posted by GitBox <gi...@apache.org>.
sk0x50 commented on a change in pull request #9248:
URL: https://github.com/apache/ignite/pull/9248#discussion_r670702096
##########
File path: modules/control-utility/src/test/java/org/apache/ignite/util/GridCommandHandlerPropertiesTest.java
##########
@@ -197,4 +199,47 @@ public void testPropertyDefaultQueryTimeout() {
)
);
}
+
+ /**
+ * Check the set command for property 'history.rebalance.threshold'.
+ */
+ @Test
+ public void testPropertyWalRebalanceThreshold() {
+ assertDistributedPropertyEquals(HISTORICAL_REBALANCE_THRESHOLD_DMS_KEY, DFLT_PDS_WAL_REBALANCE_THRESHOLD, true);
+
+ int newVal = DFLT_PDS_WAL_REBALANCE_THRESHOLD * 2;
+
+ assertEquals(
+ EXIT_CODE_OK,
+ execute(
+ "--property", "set",
+ "--name", HISTORICAL_REBALANCE_THRESHOLD_DMS_KEY,
+ "--val", Integer.toString(newVal)
+ )
+ );
+
+ assertDistributedPropertyEquals(HISTORICAL_REBALANCE_THRESHOLD_DMS_KEY, newVal, true);
+ }
+
+ /**
+ * Validates that distributed property has specified value across all nodes.
+ *
+ * @param propName Distributed property name.
+ * @param expected Expected property value.
+ * @param onlyServerMode Ignore client nodes.
+ * @param <T> Property type.
+ */
+ private <T extends Serializable> void assertDistributedPropertyEquals(String propName, T expected, boolean onlyServerMode) {
+ for (Ignite ign : G.allGrids()) {
+ IgniteEx ignEx = (IgniteEx) ign;
+
+ if (onlyServerMode && ign.configuration().isClientMode())
+ continue;
+
+ DistributedChangeableProperty<Serializable> prop =
+ ignEx.context().distributedConfiguration().property(propName);
+
+ assertEquals(prop.get(), expected);
Review comment:
It would be nice to add a human-readable message here that includes a node name at least. Something like as follows:
```
assertEquals(
"Validation has failed on the cluster node [name=" + ignEx.config().getIgniteInstanceName + ']',
prop.get(),
expected);
```
--
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
[GitHub] [ignite] sk0x50 commented on a change in pull request #9248: IGNITE-14728 Change IGNITE_PDS_WAL_REBALANCE_THRESHOLD from System property to Distributed property.
Posted by GitBox <gi...@apache.org>.
sk0x50 commented on a change in pull request #9248:
URL: https://github.com/apache/ignite/pull/9248#discussion_r670700901
##########
File path: modules/core/src/main/java/org/apache/ignite/IgniteSystemProperties.java
##########
@@ -1113,7 +1114,9 @@
/**
* WAL rebalance threshold.
+ * @deprecated use Distributed MetaStorage property {@link GridCacheDatabaseSharedManager#HISTORICAL_REBALANCE_THRESHOLD_DMS_KEY}.
Review comment:
It does not seem correct to provide a link to the internal package/class/field. IgniteSystemProperties is a kind of public API, I think. Perhaps, we need to create a separate class that should contain names of public distributed properties. What do you think?
--
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
[GitHub] [ignite] Erixonich commented on a change in pull request #9248: IGNITE-14728 Change IGNITE_PDS_WAL_REBALANCE_THRESHOLD from System property to Distributed property.
Posted by GitBox <gi...@apache.org>.
Erixonich commented on a change in pull request #9248:
URL: https://github.com/apache/ignite/pull/9248#discussion_r671169457
##########
File path: modules/core/src/main/java/org/apache/ignite/IgniteSystemProperties.java
##########
@@ -1113,7 +1114,9 @@
/**
* WAL rebalance threshold.
+ * @deprecated use Distributed MetaStorage property {@link GridCacheDatabaseSharedManager#HISTORICAL_REBALANCE_THRESHOLD_DMS_KEY}.
Review comment:
changed to {@code history.rebalance.threshold}
--
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
[GitHub] [ignite] sk0x50 commented on a change in pull request #9248: IGNITE-14728 Change IGNITE_PDS_WAL_REBALANCE_THRESHOLD from System property to Distributed property.
Posted by GitBox <gi...@apache.org>.
sk0x50 commented on a change in pull request #9248:
URL: https://github.com/apache/ignite/pull/9248#discussion_r670702687
##########
File path: modules/control-utility/src/test/java/org/apache/ignite/util/GridCommandHandlerPropertiesTest.java
##########
@@ -197,4 +199,47 @@ public void testPropertyDefaultQueryTimeout() {
)
);
}
+
+ /**
+ * Check the set command for property 'history.rebalance.threshold'.
+ */
+ @Test
+ public void testPropertyWalRebalanceThreshold() {
+ assertDistributedPropertyEquals(HISTORICAL_REBALANCE_THRESHOLD_DMS_KEY, DFLT_PDS_WAL_REBALANCE_THRESHOLD, true);
+
+ int newVal = DFLT_PDS_WAL_REBALANCE_THRESHOLD * 2;
+
+ assertEquals(
+ EXIT_CODE_OK,
+ execute(
+ "--property", "set",
+ "--name", HISTORICAL_REBALANCE_THRESHOLD_DMS_KEY,
+ "--val", Integer.toString(newVal)
+ )
+ );
+
+ assertDistributedPropertyEquals(HISTORICAL_REBALANCE_THRESHOLD_DMS_KEY, newVal, true);
+ }
+
+ /**
+ * Validates that distributed property has specified value across all nodes.
+ *
+ * @param propName Distributed property name.
+ * @param expected Expected property value.
+ * @param onlyServerMode Ignore client nodes.
+ * @param <T> Property type.
+ */
+ private <T extends Serializable> void assertDistributedPropertyEquals(String propName, T expected, boolean onlyServerMode) {
Review comment:
Do we really need the `onlyServerMode` parameter? It seems that it is always true.
--
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
[GitHub] [ignite] Erixonich commented on a change in pull request #9248: IGNITE-14728 Change IGNITE_PDS_WAL_REBALANCE_THRESHOLD from System property to Distributed property.
Posted by GitBox <gi...@apache.org>.
Erixonich commented on a change in pull request #9248:
URL: https://github.com/apache/ignite/pull/9248#discussion_r671167571
##########
File path: modules/control-utility/src/test/java/org/apache/ignite/util/GridCommandHandlerPropertiesTest.java
##########
@@ -197,4 +199,47 @@ public void testPropertyDefaultQueryTimeout() {
)
);
}
+
+ /**
+ * Check the set command for property 'history.rebalance.threshold'.
+ */
+ @Test
+ public void testPropertyWalRebalanceThreshold() {
+ assertDistributedPropertyEquals(HISTORICAL_REBALANCE_THRESHOLD_DMS_KEY, DFLT_PDS_WAL_REBALANCE_THRESHOLD, true);
+
+ int newVal = DFLT_PDS_WAL_REBALANCE_THRESHOLD * 2;
+
+ assertEquals(
+ EXIT_CODE_OK,
+ execute(
+ "--property", "set",
+ "--name", HISTORICAL_REBALANCE_THRESHOLD_DMS_KEY,
+ "--val", Integer.toString(newVal)
+ )
+ );
+
+ assertDistributedPropertyEquals(HISTORICAL_REBALANCE_THRESHOLD_DMS_KEY, newVal, true);
+ }
+
+ /**
+ * Validates that distributed property has specified value across all nodes.
+ *
+ * @param propName Distributed property name.
+ * @param expected Expected property value.
+ * @param onlyServerMode Ignore client nodes.
+ * @param <T> Property type.
+ */
+ private <T extends Serializable> void assertDistributedPropertyEquals(String propName, T expected, boolean onlyServerMode) {
Review comment:
removed unused parameter
--
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
[GitHub] [ignite] asfgit closed pull request #9248: IGNITE-14728 Change IGNITE_PDS_WAL_REBALANCE_THRESHOLD from System property to Distributed property.
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #9248:
URL: https://github.com/apache/ignite/pull/9248
--
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