[GitHub] [zookeeper] eolivelli commented on a change in pull request #1581: ZOOKEEPER-3781: Create snapshots on followers when snapshot.trust.emp…

[GitBox <gi...@apache.org>]

eolivelli commented on a change in pull request #1581:
URL: https://github.com/apache/zookeeper/pull/1581#discussion_r560066468



##########
File path: zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java
##########
@@ -530,6 +530,10 @@ public void takeSnapshot(boolean syncSnap) {
         ServerMetrics.getMetrics().SNAPSHOT_TIME.add(elapsed);
     }
 
+    public boolean shouldForceWriteInitialSnapshotDuring34Migration() {

Review comment:
       there is no need to add 'During34Migration', we can just leave a javadoc

##########
File path: zookeeper-server/src/test/java/org/apache/zookeeper/test/EmptiedSnapshotRecoveryTest.java
##########
@@ -143,6 +144,48 @@ public void testRestoreWithTrustedEmptySnapFiles() throws Exception {
         runTest(false, true);
     }
 
+    @Test
+    public void testRestoreWithTrustedEmptySnapFilesWhenFollowing() throws Exception {
+        QuorumUtil qu = new QuorumUtil(1);
+        try {
+            qu.startAll();
+            String connString = qu.getConnectionStringForServer(1);
+            try (ZooKeeper zk = new ZooKeeper(connString, CONNECTION_TIMEOUT, this)) {
+                for (int i = 0; i < N_TRANSACTIONS; i++) {
+                    zk.create("/node-" + i, new byte[0], Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT);
+                }
+            }
+            int leaderIndex = qu.getLeaderServer();
+            //Shut down the cluster and delete the snapshots from the followers
+            for (int i = 1; i <= qu.ALL; i++) {
+                qu.shutdown(i);
+                if (i != leaderIndex) {
+                    FileTxnSnapLog txnLogFactory = qu.getPeer(i).peer.getTxnFactory();
+                    List<File> snapshots = txnLogFactory.findNRecentSnapshots(10);
+                    assertTrue(snapshots.size() > 0, "We have a snapshot to corrupt");
+                    for (File file : snapshots) {
+                        Files.deleteIfExists(file.toPath());

Review comment:
       we are sure that the file exists, we can just use 'delete', correct ?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org