You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "vladnovoren (via GitHub)" <gi...@apache.org> on 2025/04/08 12:22:25 UTC
[PR] IGNITE-24535 Fix of remote nodes startup. [ignite]
vladnovoren opened a new pull request, #11986:
URL: https://github.com/apache/ignite/pull/11986
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
Re: [PR] IGNITE-24535 Fix of remote nodes startup. [ignite]
Posted by "chesnokoff (via GitHub)" <gi...@apache.org>.
chesnokoff commented on code in PR #11986:
URL: https://github.com/apache/ignite/pull/11986#discussion_r2077262127
##########
modules/compatibility/src/test/java/org/apache/ignite/compatibility/persistence/SnapshotCompatibilityTest.java:
##########
@@ -142,19 +134,35 @@ public void setUp() {
/** */
@Test
public void testSnapshotRestore() throws Exception {
+ if (incSnp) {
+ assumeFalse("Incremental snapshots for cache dump not supported", cacheDump);
+
+ assumeTrue("Incremental snapshots require same consistentID", customConsId);
+
+ assumeTrue("https://issues.apache.org/jira/browse/IGNITE-25096", oldNodesCnt == 1);
+ }
+
try {
+ for (int i = 1; i < oldNodesCnt; ++i) {
+ startGrid(
+ i,
+ OLD_IGNITE_VERSION,
+ new ConfigurationClosure(incSnp, consId(i), customSnpPath, true, cacheGrpInfo)
+ );
+ }
+
startGrid(
oldNodesCnt,
OLD_IGNITE_VERSION,
- new ConfigurationClosure(incSnp, consId, customSnpPath, true, cacheGrpInfo),
+ new ConfigurationClosure(incSnp, consId(oldNodesCnt), customSnpPath, true, cacheGrpInfo),
new CreateSnapshotClosure(incSnp, cacheDump, cacheGrpInfo)
);
Review Comment:
What about just passing `null` as fourth argument of `startGrid` when it needed? Then you could merge this two types of grid starting
--
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
Re: [PR] IGNITE-24535 Fix of remote nodes startup. [ignite]
Posted by "chesnokoff (via GitHub)" <gi...@apache.org>.
chesnokoff commented on code in PR #11986:
URL: https://github.com/apache/ignite/pull/11986#discussion_r2073831451
##########
modules/compatibility/src/test/java/org/apache/ignite/compatibility/persistence/SnapshotCompatibilityTest.java:
##########
@@ -284,6 +289,11 @@ private static String calcValue(String cacheName, int key) {
return cacheName + "-organization-" + key;
}
+ /** */
+ private static String consId(boolean custom, int nodeIdx) {
+ return custom ? "node-" + nodeIdx : null;
+ }
+
Review Comment:
Make it non-static and use customConsId field
--
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
Re: [PR] IGNITE-24535 Fix of remote nodes startup. [ignite]
Posted by "nizhikov (via GitHub)" <gi...@apache.org>.
nizhikov commented on code in PR #11986:
URL: https://github.com/apache/ignite/pull/11986#discussion_r2052204404
##########
modules/compatibility/src/test/java/org/apache/ignite/compatibility/persistence/SnapshotCompatibilityTest.java:
##########
@@ -142,19 +135,33 @@ public void setUp() {
/** */
@Test
public void testSnapshotRestore() throws Exception {
+ assumeFalse("Incremental snapshots for cache dump not supported", incSnp && cacheDump);
+
+ assumeFalse("Incremental snapshots require same consistentID", incSnp && !customConsId);
+
+ assumeFalse("https://issues.apache.org/jira/browse/IGNITE-25096", incSnp && oldNodesCnt == 3);
+
try {
+ for (int i = 1; i < oldNodesCnt; ++i) {
+ startGrid(
+ i,
+ OLD_IGNITE_VERSION,
+ new ConfigurationClosure(incSnp, consId(customConsId, i), customSnpPath, true, cacheGrpInfo)
+ );
+ }
+
startGrid(
oldNodesCnt,
OLD_IGNITE_VERSION,
- new ConfigurationClosure(incSnp, consId, customSnpPath, true, cacheGrpInfo),
+ new ConfigurationClosure(incSnp, consId(customConsId, oldNodesCnt), customSnpPath, true, cacheGrpInfo),
new CreateSnapshotClosure(incSnp, cacheDump, cacheGrpInfo)
);
stopAllGrids();
cleanPersistenceDir(true);
- IgniteEx node = startGrid(currentIgniteConfiguration(incSnp, consId, customSnpPath));
+ IgniteEx node = startGrid(currentIgniteConfiguration(incSnp, consId(customConsId, 1), customSnpPath));
Review Comment:
We must start same count of nodes for old version and current version.
--
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
Re: [PR] IGNITE-24535 Fix of remote nodes startup. [ignite]
Posted by "nizhikov (via GitHub)" <gi...@apache.org>.
nizhikov commented on code in PR #11986:
URL: https://github.com/apache/ignite/pull/11986#discussion_r2044236534
##########
modules/compatibility/src/test/java/org/apache/ignite/compatibility/persistence/SnapshotCompatibilityTest.java:
##########
@@ -142,19 +135,35 @@ public void setUp() {
/** */
@Test
public void testSnapshotRestore() throws Exception {
+ assumeFalse("User must not restore both snapshots and cache dumps simultaneously", incSnp && cacheDump);
+
+ // See <a href="https://issues.apache.org/jira/browse/IGNITE-23222">...</a>
+ assumeFalse("Restoring incremental snapshots if consistent ID is null is fixed in 2.17.0", incSnp && !customConsId);
+
+ // See <a href="https://issues.apache.org/jira/browse/IGNITE-25096">...</a>
+ assumeFalse("Restoring incremental snapshots on different topology is unsopported", incSnp && oldNodesCnt == 3);
Review Comment:
"https://issues.apache.org/jira/browse/IGNITE-25096"
##########
modules/compatibility/src/test/java/org/apache/ignite/compatibility/persistence/SnapshotCompatibilityTest.java:
##########
@@ -142,19 +135,35 @@ public void setUp() {
/** */
@Test
public void testSnapshotRestore() throws Exception {
+ assumeFalse("User must not restore both snapshots and cache dumps simultaneously", incSnp && cacheDump);
+
+ // See <a href="https://issues.apache.org/jira/browse/IGNITE-23222">...</a>
+ assumeFalse("Restoring incremental snapshots if consistent ID is null is fixed in 2.17.0", incSnp && !customConsId);
+
+ // See <a href="https://issues.apache.org/jira/browse/IGNITE-25096">...</a>
+ assumeFalse("Restoring incremental snapshots on different topology is unsopported", incSnp && oldNodesCnt == 3);
Review Comment:
"Restoring incremental snapshots on different topology is unsopported" -> "https://issues.apache.org/jira/browse/IGNITE-25096"
--
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
Re: [PR] IGNITE-24535 Fix of remote nodes startup. [ignite]
Posted by "chesnokoff (via GitHub)" <gi...@apache.org>.
chesnokoff commented on code in PR #11986:
URL: https://github.com/apache/ignite/pull/11986#discussion_r2073724491
##########
modules/compatibility/src/test/java/org/apache/ignite/compatibility/persistence/SnapshotCompatibilityTest.java:
##########
@@ -142,19 +133,33 @@ public void setUp() {
/** */
@Test
public void testSnapshotRestore() throws Exception {
+ assumeFalse("Incremental snapshots for cache dump not supported", incSnp && cacheDump);
+
+ assumeFalse("Incremental snapshots require same consistentID", incSnp && !customConsId);
+
+ assumeFalse("https://issues.apache.org/jira/browse/IGNITE-25096", incSnp && oldNodesCnt == 3);
Review Comment:
Replace with TODO: IGNITE-25096
--
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
Re: [PR] IGNITE-24535 Fix of remote nodes startup. [ignite]
Posted by "nizhikov (via GitHub)" <gi...@apache.org>.
nizhikov commented on code in PR #11986:
URL: https://github.com/apache/ignite/pull/11986#discussion_r2052203595
##########
modules/compatibility/src/test/java/org/apache/ignite/compatibility/persistence/SnapshotCompatibilityTest.java:
##########
@@ -83,52 +84,44 @@ public class SnapshotCompatibilityTest extends IgniteCompatibilityAbstractTest {
private static final String CUSTOM_SNP_RELATIVE_PATH = "ex_snapshots";
/** */
- private static final String CONSISTENT_ID = UUID.randomUUID().toString();
-
- /** */
- @Parameterized.Parameter
+ @Parameter
public boolean incSnp;
/** */
- @Parameterized.Parameter(1)
- @Nullable public String consId;
+ @Parameter(1)
+ public boolean customConsId;
/** */
- @Parameterized.Parameter(2)
+ @Parameter(2)
public int oldNodesCnt;
Review Comment:
oldNodesCnt -> nodesCnt
--
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
Re: [PR] IGNITE-24535 Fix of remote nodes startup. [ignite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #11986:
URL: https://github.com/apache/ignite/pull/11986#issuecomment-2859522351
## [](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=11986) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_ignite&pullRequest=11986&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_ignite&pullRequest=11986&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_ignite&pullRequest=11986&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_ignite&pullRequest=11986&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_ignite&pullRequest=11986&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=11986)
--
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
Re: [PR] IGNITE-24535 Fix of remote nodes startup. [ignite]
Posted by "nizhikov (via GitHub)" <gi...@apache.org>.
nizhikov commented on code in PR #11986:
URL: https://github.com/apache/ignite/pull/11986#discussion_r2044233826
##########
modules/compatibility/src/test/java/org/apache/ignite/compatibility/persistence/SnapshotCompatibilityTest.java:
##########
@@ -142,19 +135,35 @@ public void setUp() {
/** */
@Test
public void testSnapshotRestore() throws Exception {
+ assumeFalse("User must not restore both snapshots and cache dumps simultaneously", incSnp && cacheDump);
Review Comment:
"User must not restore both snapshots and cache dumps simultaneously" -> "Incremental snapshots for cache dump not supported"
--
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
Re: [PR] IGNITE-24535 Fix of remote nodes startup. [ignite]
Posted by "vladnovoren (via GitHub)" <gi...@apache.org>.
vladnovoren commented on code in PR #11986:
URL: https://github.com/apache/ignite/pull/11986#discussion_r2053892134
##########
modules/compatibility/src/test/java/org/apache/ignite/compatibility/persistence/SnapshotCompatibilityTest.java:
##########
@@ -142,19 +135,33 @@ public void setUp() {
/** */
@Test
public void testSnapshotRestore() throws Exception {
+ assumeFalse("Incremental snapshots for cache dump not supported", incSnp && cacheDump);
+
+ assumeFalse("Incremental snapshots require same consistentID", incSnp && !customConsId);
+
+ assumeFalse("https://issues.apache.org/jira/browse/IGNITE-25096", incSnp && oldNodesCnt == 3);
+
try {
+ for (int i = 1; i < oldNodesCnt; ++i) {
+ startGrid(
+ i,
+ OLD_IGNITE_VERSION,
+ new ConfigurationClosure(incSnp, consId(customConsId, i), customSnpPath, true, cacheGrpInfo)
+ );
+ }
+
startGrid(
oldNodesCnt,
OLD_IGNITE_VERSION,
- new ConfigurationClosure(incSnp, consId, customSnpPath, true, cacheGrpInfo),
+ new ConfigurationClosure(incSnp, consId(customConsId, oldNodesCnt), customSnpPath, true, cacheGrpInfo),
new CreateSnapshotClosure(incSnp, cacheDump, cacheGrpInfo)
);
stopAllGrids();
cleanPersistenceDir(true);
- IgniteEx node = startGrid(currentIgniteConfiguration(incSnp, consId, customSnpPath));
+ IgniteEx node = startGrid(currentIgniteConfiguration(incSnp, consId(customConsId, 1), customSnpPath));
Review Comment:
From ticket:
...
Node configurations must include:
- consistent id provided (with the "-" and other symbols the will be replaced by the U.maskForFileName method).
- consistentId == null.
- **Snapshot restoration on the different cluster (with the different topology).**
--
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
Re: [PR] IGNITE-24535 Fix of remote nodes startup. [ignite]
Posted by "nizhikov (via GitHub)" <gi...@apache.org>.
nizhikov commented on code in PR #11986:
URL: https://github.com/apache/ignite/pull/11986#discussion_r2052203876
##########
modules/compatibility/src/test/java/org/apache/ignite/compatibility/persistence/SnapshotCompatibilityTest.java:
##########
@@ -83,52 +84,44 @@ public class SnapshotCompatibilityTest extends IgniteCompatibilityAbstractTest {
private static final String CUSTOM_SNP_RELATIVE_PATH = "ex_snapshots";
/** */
- private static final String CONSISTENT_ID = UUID.randomUUID().toString();
-
- /** */
- @Parameterized.Parameter
+ @Parameter
public boolean incSnp;
/** */
- @Parameterized.Parameter(1)
- @Nullable public String consId;
+ @Parameter(1)
Review Comment:
Please, revert import of `Parameter` class to make PR more clear.
--
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
Re: [PR] IGNITE-24535 Fix of remote nodes startup. [ignite]
Posted by "chesnokoff (via GitHub)" <gi...@apache.org>.
chesnokoff commented on code in PR #11986:
URL: https://github.com/apache/ignite/pull/11986#discussion_r2073724491
##########
modules/compatibility/src/test/java/org/apache/ignite/compatibility/persistence/SnapshotCompatibilityTest.java:
##########
@@ -142,19 +133,33 @@ public void setUp() {
/** */
@Test
public void testSnapshotRestore() throws Exception {
+ assumeFalse("Incremental snapshots for cache dump not supported", incSnp && cacheDump);
+
+ assumeFalse("Incremental snapshots require same consistentID", incSnp && !customConsId);
+
+ assumeFalse("https://issues.apache.org/jira/browse/IGNITE-25096", incSnp && oldNodesCnt == 3);
Review Comment:
TODO?
--
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
Re: [PR] IGNITE-24535 Fix of remote nodes startup. [ignite]
Posted by "chesnokoff (via GitHub)" <gi...@apache.org>.
chesnokoff commented on code in PR #11986:
URL: https://github.com/apache/ignite/pull/11986#discussion_r2073724491
##########
modules/compatibility/src/test/java/org/apache/ignite/compatibility/persistence/SnapshotCompatibilityTest.java:
##########
@@ -142,19 +133,33 @@ public void setUp() {
/** */
@Test
public void testSnapshotRestore() throws Exception {
+ assumeFalse("Incremental snapshots for cache dump not supported", incSnp && cacheDump);
+
+ assumeFalse("Incremental snapshots require same consistentID", incSnp && !customConsId);
+
+ assumeFalse("https://issues.apache.org/jira/browse/IGNITE-25096", incSnp && oldNodesCnt == 3);
Review Comment:
Replace with TODO: IGNITE-25096
--
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
Re: [PR] IGNITE-24535 Fix of remote nodes startup. [ignite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #11986:
URL: https://github.com/apache/ignite/pull/11986#issuecomment-2858820908
## [](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=11986) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_ignite&pullRequest=11986&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_ignite&pullRequest=11986&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_ignite&pullRequest=11986&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_ignite&pullRequest=11986&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_ignite&pullRequest=11986&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_ignite&pullRequest=11986)
--
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
Re: [PR] IGNITE-24535 Fix of remote nodes startup. [ignite]
Posted by "nizhikov (via GitHub)" <gi...@apache.org>.
nizhikov commented on code in PR #11986:
URL: https://github.com/apache/ignite/pull/11986#discussion_r2044055948
##########
modules/compatibility/src/test/java/org/apache/ignite/compatibility/persistence/SnapshotCompatibilityTest.java:
##########
@@ -83,52 +82,50 @@ public class SnapshotCompatibilityTest extends IgniteCompatibilityAbstractTest {
private static final String CUSTOM_SNP_RELATIVE_PATH = "ex_snapshots";
/** */
- private static final String CONSISTENT_ID = UUID.randomUUID().toString();
-
- /** */
- @Parameterized.Parameter
+ @Parameter
public boolean incSnp;
/** */
- @Parameterized.Parameter(1)
- @Nullable public String consId;
+ @Parameter(1)
+ public boolean customConsId;
/** */
- @Parameterized.Parameter(2)
+ @Parameter(2)
public int oldNodesCnt;
/** */
- @Parameterized.Parameter(3)
+ @Parameter(3)
public boolean cacheDump;
/** */
- @Parameterized.Parameter(4)
+ @Parameter(4)
public boolean customSnpPath;
/** */
- @Parameterized.Parameter(5)
+ @Parameter(5)
public boolean testCacheGrp;
/** */
private CacheGroupInfo cacheGrpInfo;
/**
- * The test is parameterized by whether an incremental snapshot is taken and by consistentId.
- * Restore incremental snapshot if consistentId is null is fixed in 2.17.0, see here https://issues.apache.org/jira/browse/IGNITE-23222.
- * Also restoring cache dump and any kind of snapshot is pointless.
+ * Restore incremental snapshot if consistent ID is null is fixed in 2.17.0, see here
+ * <a href="https://issues.apache.org/jira/browse/IGNITE-23222">...</a>. Restore of an incremental snapshot doesn't work for different
+ * topology. Also restoring cache dump and any kind of snapshot is pointless.
*/
- @Parameters(name = "incrementalSnp={0}, consistentID={1}, oldNodesCnt={2}, cacheDump={3}, customSnpPath={4}, testCacheGrp={5}")
+ @Parameters(name = "incSnp={0}, customConsId={1}, oldNodesCnt={2}, cacheDump={3}, customSnpPath={4}, testCacheGrp={5}")
public static Collection<Object[]> data() {
List<Object[]> data = new ArrayList<>();
for (boolean incSnp : Arrays.asList(true, false))
- for (String consId : Arrays.asList(CONSISTENT_ID, null))
+ for (boolean customConsId: Arrays.asList(true, false))
for (int oldNodesCnt : Arrays.asList(1, 3))
for (boolean cacheDump : Arrays.asList(true, false))
for (boolean customSnpPath : Arrays.asList(true, false))
for (boolean testCacheGrp : Arrays.asList(true, false))
- if ((!incSnp || !cacheDump) && (!incSnp || consId != null))
- data.add(new Object[]{incSnp, consId, oldNodesCnt, cacheDump, customSnpPath, testCacheGrp});
+ // see <a href="https://issues.apache.org/jira/browse/IGNITE-25096">...</a>
+ if (!incSnp || (!cacheDump && customConsId && oldNodesCnt == 1))
Review Comment:
This clause looks very complicated.
Let's:
1. Simplify it. What cases do you want to skip?
2. Mute those cases in test with the `assumeFalse` like:
```
@Test
public void testSnapshotRestore() throws Exception {
assumeFalse("Description here", ...);
```
--
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
Re: [PR] IGNITE-24535 Fix of remote nodes startup. [ignite]
Posted by "nizhikov (via GitHub)" <gi...@apache.org>.
nizhikov commented on code in PR #11986:
URL: https://github.com/apache/ignite/pull/11986#discussion_r2044234469
##########
modules/compatibility/src/test/java/org/apache/ignite/compatibility/persistence/SnapshotCompatibilityTest.java:
##########
@@ -142,19 +135,35 @@ public void setUp() {
/** */
@Test
public void testSnapshotRestore() throws Exception {
+ assumeFalse("User must not restore both snapshots and cache dumps simultaneously", incSnp && cacheDump);
+
+ // See <a href="https://issues.apache.org/jira/browse/IGNITE-23222">...</a>
+ assumeFalse("Restoring incremental snapshots if consistent ID is null is fixed in 2.17.0", incSnp && !customConsId);
Review Comment:
"Restoring incremental snapshots if consistent ID is null is fixed in 2.17.0" -> "https://issues.apache.org/jira/browse/IGNITE-23222"
--
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
Re: [PR] IGNITE-24535 Fix of remote nodes startup. [ignite]
Posted by "nizhikov (via GitHub)" <gi...@apache.org>.
nizhikov commented on code in PR #11986:
URL: https://github.com/apache/ignite/pull/11986#discussion_r2044063493
##########
modules/compatibility/src/test/java/org/apache/ignite/compatibility/persistence/SnapshotCompatibilityTest.java:
##########
@@ -143,18 +140,26 @@ public void setUp() {
@Test
public void testSnapshotRestore() throws Exception {
try {
+ for (int i = 1; i < oldNodesCnt; ++i) {
+ startGrid(
+ i,
+ OLD_IGNITE_VERSION,
+ new ConfigurationClosure(incSnp, consId(customConsId, i), customSnpPath, true, cacheGrpInfo)
+ );
+ }
+
startGrid(
- oldNodesCnt,
- OLD_IGNITE_VERSION,
- new ConfigurationClosure(incSnp, consId, customSnpPath, true, cacheGrpInfo),
- new CreateSnapshotClosure(incSnp, cacheDump, cacheGrpInfo)
+ oldNodesCnt,
Review Comment:
We use 4 character indentation.
##########
modules/compatibility/src/test/java/org/apache/ignite/compatibility/persistence/SnapshotCompatibilityTest.java:
##########
@@ -143,18 +140,26 @@ public void setUp() {
@Test
public void testSnapshotRestore() throws Exception {
try {
+ for (int i = 1; i < oldNodesCnt; ++i) {
+ startGrid(
+ i,
Review Comment:
We use 4 character indentation.
--
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
Re: [PR] IGNITE-24535 Fix of remote nodes startup. [ignite]
Posted by "chesnokoff (via GitHub)" <gi...@apache.org>.
chesnokoff commented on code in PR #11986:
URL: https://github.com/apache/ignite/pull/11986#discussion_r2073724491
##########
modules/compatibility/src/test/java/org/apache/ignite/compatibility/persistence/SnapshotCompatibilityTest.java:
##########
@@ -142,19 +133,33 @@ public void setUp() {
/** */
@Test
public void testSnapshotRestore() throws Exception {
+ assumeFalse("Incremental snapshots for cache dump not supported", incSnp && cacheDump);
+
+ assumeFalse("Incremental snapshots require same consistentID", incSnp && !customConsId);
+
+ assumeFalse("https://issues.apache.org/jira/browse/IGNITE-25096", incSnp && oldNodesCnt == 3);
Review Comment:
Replace link with text description
--
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
Re: [PR] IGNITE-24535 Fix of remote nodes startup. [ignite]
Posted by "chesnokoff (via GitHub)" <gi...@apache.org>.
chesnokoff commented on code in PR #11986:
URL: https://github.com/apache/ignite/pull/11986#discussion_r2073829913
##########
modules/compatibility/src/test/java/org/apache/ignite/compatibility/persistence/SnapshotCompatibilityTest.java:
##########
@@ -142,19 +133,33 @@ public void setUp() {
/** */
@Test
public void testSnapshotRestore() throws Exception {
+ assumeFalse("Incremental snapshots for cache dump not supported", incSnp && cacheDump);
+
+ assumeFalse("Incremental snapshots require same consistentID", incSnp && !customConsId);
+
+ assumeFalse("https://issues.apache.org/jira/browse/IGNITE-25096", incSnp && oldNodesCnt == 3);
+
Review Comment:
if (incSnp) {<assumes>}
##########
modules/compatibility/src/test/java/org/apache/ignite/compatibility/persistence/SnapshotCompatibilityTest.java:
##########
@@ -142,19 +133,33 @@ public void setUp() {
/** */
@Test
public void testSnapshotRestore() throws Exception {
+ assumeFalse("Incremental snapshots for cache dump not supported", incSnp && cacheDump);
+
+ assumeFalse("Incremental snapshots require same consistentID", incSnp && !customConsId);
+
+ assumeFalse("https://issues.apache.org/jira/browse/IGNITE-25096", incSnp && oldNodesCnt == 3);
+
Review Comment:
if (incSnp) {\<assumes\>}
--
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
Re: [PR] IGNITE-24535 Fix of remote nodes startup. [ignite]
Posted by "nizhikov (via GitHub)" <gi...@apache.org>.
nizhikov merged PR #11986:
URL: https://github.com/apache/ignite/pull/11986
--
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