timoninmaxim commented on code in PR #11588:
URL: https://github.com/apache/ignite/pull/11588#discussion_r1800604836
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/IgniteSnapshotManager.java:
##########
@@ -2289,25 +2292,23 @@ else if (grps.isEmpty())
Set<UUID> bltNodeIds =
new HashSet<>(F.viewReadOnly(srvNodes, F.node2id(), (node) ->
CU.baselineNode(node, clusterState)));
- startSnpProc.start(snpFut0.rqId, new SnapshotOperationRequest(
- snpFut0.rqId,
- cctx.localNodeId(),
- name,
- snpPath,
- grps,
- bltNodeIds,
- incremental,
- incIdx,
- onlyPrimary,
- dump,
- compress,
- encrypt
- ));
+ var snpOpReq = new SnapshotOperationRequest(
+ snpFut0.rqId,
+ cctx.localNodeId(),
+ name,
+ snpPath,
+ grps,
+ bltNodeIds,
+ incremental,
+ incIdx,
+ onlyPrimary,
+ dump,
+ compress,
+ encrypt
+ );
+ startSnpProc.start(snpFut0.rqId, snpOpReq);
Review Comment:
Add blank line before this statement according to Ignite coding guidelines
[1]
[1]
https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-SemanticUnits
##########
modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/IgniteSnapshotManagerSelfTest.java:
##########
@@ -581,6 +581,40 @@ public void testSnapshotThreadPoolSizeUsage() throws
Exception {
assertEquals(snapshotThreadPoolSize.longValue(), snpRunningThreads);
}
+ /**
+ * Tests that full-copy snapshot logs correctly.
+ *
+ * @throws Exception If fails.
+ * */
+ @Test
+ public void testSnapshotCreationLog() throws Exception {
+ if (listenLog == null)
+ listenLog = new ListeningTestLogger(log);
+
+ final int ENTRIES_CNT = 4;
Review Comment:
ENTRIES_CNT -> entriesCnt. Use upper case for class-level constants only [1]
[1]
https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines#CodingGuidelines-Naming
##########
modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/IgniteSnapshotManagerSelfTest.java:
##########
@@ -581,6 +581,40 @@ public void testSnapshotThreadPoolSizeUsage() throws
Exception {
assertEquals(snapshotThreadPoolSize.longValue(), snpRunningThreads);
}
+ /**
+ * Tests that full-copy snapshot logs correctly.
+ *
+ * @throws Exception If fails.
+ * */
+ @Test
+ public void testSnapshotCreationLog() throws Exception {
+ if (listenLog == null)
+ listenLog = new ListeningTestLogger(log);
+
+ final int ENTRIES_CNT = 4;
+
+ LogListener matchLsnr1 = LogListener.matches("Cluster-wide snapshot
operation started: ").build();
+ listenLog.registerListener(matchLsnr1);
+
+ LogListener matchLsnr2 = LogListener.matches("incremental=false,
incIdx=-1").build();
+ listenLog.registerListener(matchLsnr2);
+
+ LogListener noMatchLsnr = LogListener.matches("incremental=true,
incIdx=-1").build();
Review Comment:
Let's add similar check for incremental snapshot in this test.
##########
modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/IgniteSnapshotManagerSelfTest.java:
##########
@@ -581,6 +581,40 @@ public void testSnapshotThreadPoolSizeUsage() throws
Exception {
assertEquals(snapshotThreadPoolSize.longValue(), snpRunningThreads);
}
+ /**
+ * Tests that full-copy snapshot logs correctly.
+ *
+ * @throws Exception If fails.
+ * */
+ @Test
+ public void testSnapshotCreationLog() throws Exception {
+ if (listenLog == null)
Review Comment:
You don't need this check. `listenLog` must be new for every test to avoid
side effects.
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/snapshot/IgniteSnapshotManager.java:
##########
@@ -2289,25 +2292,23 @@ else if (grps.isEmpty())
Set<UUID> bltNodeIds =
new HashSet<>(F.viewReadOnly(srvNodes, F.node2id(), (node) ->
CU.baselineNode(node, clusterState)));
- startSnpProc.start(snpFut0.rqId, new SnapshotOperationRequest(
- snpFut0.rqId,
- cctx.localNodeId(),
- name,
- snpPath,
- grps,
- bltNodeIds,
- incremental,
- incIdx,
- onlyPrimary,
- dump,
- compress,
- encrypt
- ));
+ var snpOpReq = new SnapshotOperationRequest(
Review Comment:
Do not use `var` until you have great reason for that. Style guide for using
`var` is here [1]. In current situation is better to use explicit type
`SnapshotOperationRequest`. It's name is simple and the variable is used twice.
[1] https://openjdk.org/projects/amber/guides/lvti-style-guide
--
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]
