From 0e23797218ce30420968d0752608abb36a94976a Mon Sep 17 00:00:00 2001 From: wenbingshen Date: Thu, 2 Feb 2023 16:27:21 +0800 Subject: [PATCH 1/5] fix indexDirs upgrade failed --- .../bookkeeper/bookie/FileSystemUpgrade.java | 7 +- .../apache/bookkeeper/bookie/UpgradeTest.java | 89 +++++++++++++++++-- 2 files changed, 88 insertions(+), 8 deletions(-) diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileSystemUpgrade.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileSystemUpgrade.java index bb6cd2404a3..f60467b1f67 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileSystemUpgrade.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileSystemUpgrade.java @@ -99,7 +99,12 @@ public boolean accept(File dir, String name) { private static List getAllDirectories(ServerConfiguration conf) { List dirs = new ArrayList<>(); dirs.addAll(Lists.newArrayList(conf.getJournalDirs())); - Collections.addAll(dirs, conf.getLedgerDirs()); + final File[] ledgerDirs = conf.getLedgerDirs(); + final File[] indexDirs = conf.getIndexDirs(); + if (indexDirs != null && indexDirs != ledgerDirs) { + dirs.addAll(Lists.newArrayList(indexDirs)); + } + Collections.addAll(dirs, ledgerDirs); return dirs; } diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/UpgradeTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/UpgradeTest.java index b259dfd9016..da25cf962b3 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/UpgradeTest.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/UpgradeTest.java @@ -62,6 +62,28 @@ public UpgradeTest() { super(0); } + static void writeLedgerDirWithIndexDir(File ledgerDir, + File indexDir, + byte[] masterKey) + throws Exception { + long ledgerId = 1; + + File fn = new File(indexDir, IndexPersistenceMgr.getLedgerName(ledgerId)); + fn.getParentFile().mkdirs(); + FileInfo fi = new FileInfo(fn, masterKey, FileInfo.CURRENT_HEADER_VERSION); + // force creation of index file + fi.write(new ByteBuffer[]{ ByteBuffer.allocate(0) }, 0); + fi.close(true); + + long logId = 0; + ByteBuffer logfileHeader = ByteBuffer.allocate(1024); + logfileHeader.put("BKLO".getBytes()); + FileChannel logfile = new RandomAccessFile( + new File(ledgerDir, Long.toHexString(logId) + ".log"), "rw").getChannel(); + logfile.write((ByteBuffer) logfileHeader.clear()); + logfile.close(); + } + static void writeLedgerDir(File dir, byte[] masterKey) throws Exception { @@ -122,6 +144,12 @@ static File initV1LedgerDirectory(File d) throws Exception { return d; } + static File initV1LedgerDirectoryWithIndexDir(File ledgerDir, + File indexDir) throws Exception { + writeLedgerDirWithIndexDir(ledgerDir, indexDir, "foobar".getBytes()); + return ledgerDir; + } + static void createVersion2File(File dir) throws Exception { File versionFile = new File(dir, "VERSION"); @@ -148,12 +176,20 @@ static File initV2LedgerDirectory(File d) throws Exception { return d; } - private static void testUpgradeProceedure(String zkServers, String journalDir, String ledgerDir) throws Exception { + static File initV2LedgerDirectoryWithIndexDir(File ledgerDir, File indexDir) throws Exception { + initV1LedgerDirectoryWithIndexDir(ledgerDir, indexDir); + createVersion2File(ledgerDir); + createVersion2File(indexDir); + return ledgerDir; + } + + private static void testUpgradeProceedure(String zkServers, String journalDir, String ledgerDir, String indexDir) throws Exception { ServerConfiguration conf = TestBKConfiguration.newServerConfiguration(); conf.setMetadataServiceUri("zk://" + zkServers + "/ledgers"); conf.setJournalDirName(journalDir) - .setLedgerDirNames(new String[] { ledgerDir }) - .setBookiePort(bookiePort); + .setLedgerDirNames(new String[]{ledgerDir}) + .setIndexDirName(new String[]{indexDir}) + .setBookiePort(bookiePort); Bookie b = null; try (MetadataBookieDriver metadataDriver = BookieResources.createMetadataDriver( @@ -211,21 +247,60 @@ private static void testUpgradeProceedure(String zkServers, String journalDir, S public void testUpgradeV1toCurrent() throws Exception { File journalDir = initV1JournalDirectory(tmpDirs.createNew("bookie", "journal")); File ledgerDir = initV1LedgerDirectory(tmpDirs.createNew("bookie", "ledger")); - testUpgradeProceedure(zkUtil.getZooKeeperConnectString(), journalDir.getPath(), ledgerDir.getPath()); + testUpgradeProceedure(zkUtil.getZooKeeperConnectString(), journalDir.getPath(), + ledgerDir.getPath(), ledgerDir.getPath()); + } + + @Test + public void testUpgradeV1toCurrentWithIndexDir() throws Exception { + File journalDir = initV1JournalDirectory(tmpDirs.createNew("bookie", "journal")); + File indexDir = tmpDirs.createNew("bookie", "index"); + File ledgerDir = initV1LedgerDirectoryWithIndexDir(tmpDirs.createNew("bookie", "ledger"), indexDir); + testUpgradeProceedure(zkUtil.getZooKeeperConnectString(), journalDir.getPath(), + ledgerDir.getPath(), indexDir.getPath()); } @Test public void testUpgradeV2toCurrent() throws Exception { File journalDir = initV2JournalDirectory(tmpDirs.createNew("bookie", "journal")); File ledgerDir = initV2LedgerDirectory(tmpDirs.createNew("bookie", "ledger")); - testUpgradeProceedure(zkUtil.getZooKeeperConnectString(), journalDir.getPath(), ledgerDir.getPath()); + File indexDir = tmpDirs.createNew("bookie", "index"); + testUpgradeProceedure(zkUtil.getZooKeeperConnectString(), journalDir.getPath(), + ledgerDir.getPath(), indexDir.getPath()); + } + + @Test + public void testUpgradeV2toCurrentWithIndexDir() throws Exception { + File journalDir = initV2JournalDirectory(tmpDirs.createNew("bookie", "journal")); + File indexDir = tmpDirs.createNew("bookie", "index"); + File ledgerDir = initV2LedgerDirectoryWithIndexDir(tmpDirs.createNew("bookie", "ledger"), indexDir); + testUpgradeProceedure(zkUtil.getZooKeeperConnectString(), journalDir.getPath(), + ledgerDir.getPath(), indexDir.getPath()); } @Test public void testUpgradeCurrent() throws Exception { + testUpgradeCurrent(false); + } + + @Test + public void testUpgradeCurrentWithIndexDir() throws Exception { + testUpgradeCurrent(true); + } + + public void testUpgradeCurrent(boolean hasIndexDir) throws Exception { File journalDir = initV2JournalDirectory(tmpDirs.createNew("bookie", "journal")); - File ledgerDir = initV2LedgerDirectory(tmpDirs.createNew("bookie", "ledger")); - testUpgradeProceedure(zkUtil.getZooKeeperConnectString(), journalDir.getPath(), ledgerDir.getPath()); + File ledgerDir = tmpDirs.createNew("bookie", "ledger"); + File indexDir = ledgerDir; + if (hasIndexDir) { + indexDir = tmpDirs.createNew("bookie", "index"); + initV2LedgerDirectoryWithIndexDir(ledgerDir, indexDir); + } else { + initV2LedgerDirectory(ledgerDir); + } + + testUpgradeProceedure(zkUtil.getZooKeeperConnectString(), journalDir.getPath(), + ledgerDir.getPath(), indexDir.getPath()); // Upgrade again ServerConfiguration conf = TestBKConfiguration.newServerConfiguration(); From 455927acc124499bb43e234972fdd8575043753c Mon Sep 17 00:00:00 2001 From: wenbingshen Date: Thu, 2 Feb 2023 16:40:53 +0800 Subject: [PATCH 2/5] fix checkstyle --- .../java/org/apache/bookkeeper/bookie/UpgradeTest.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/UpgradeTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/UpgradeTest.java index da25cf962b3..8b7680785f3 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/UpgradeTest.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/UpgradeTest.java @@ -183,7 +183,8 @@ static File initV2LedgerDirectoryWithIndexDir(File ledgerDir, File indexDir) thr return ledgerDir; } - private static void testUpgradeProceedure(String zkServers, String journalDir, String ledgerDir, String indexDir) throws Exception { + private static void testUpgradeProceedure(String zkServers, String journalDir, String ledgerDir, String indexDir) + throws Exception { ServerConfiguration conf = TestBKConfiguration.newServerConfiguration(); conf.setMetadataServiceUri("zk://" + zkServers + "/ledgers"); conf.setJournalDirName(journalDir) @@ -255,7 +256,8 @@ public void testUpgradeV1toCurrent() throws Exception { public void testUpgradeV1toCurrentWithIndexDir() throws Exception { File journalDir = initV1JournalDirectory(tmpDirs.createNew("bookie", "journal")); File indexDir = tmpDirs.createNew("bookie", "index"); - File ledgerDir = initV1LedgerDirectoryWithIndexDir(tmpDirs.createNew("bookie", "ledger"), indexDir); + File ledgerDir = initV1LedgerDirectoryWithIndexDir( + tmpDirs.createNew("bookie", "ledger"), indexDir); testUpgradeProceedure(zkUtil.getZooKeeperConnectString(), journalDir.getPath(), ledgerDir.getPath(), indexDir.getPath()); } @@ -273,7 +275,8 @@ public void testUpgradeV2toCurrent() throws Exception { public void testUpgradeV2toCurrentWithIndexDir() throws Exception { File journalDir = initV2JournalDirectory(tmpDirs.createNew("bookie", "journal")); File indexDir = tmpDirs.createNew("bookie", "index"); - File ledgerDir = initV2LedgerDirectoryWithIndexDir(tmpDirs.createNew("bookie", "ledger"), indexDir); + File ledgerDir = initV2LedgerDirectoryWithIndexDir( + tmpDirs.createNew("bookie", "ledger"), indexDir); testUpgradeProceedure(zkUtil.getZooKeeperConnectString(), journalDir.getPath(), ledgerDir.getPath(), indexDir.getPath()); } From 55ecd2c8ca235f0211de5bb8d8b30bdb36f6171c Mon Sep 17 00:00:00 2001 From: wenbingshen Date: Fri, 3 Feb 2023 11:16:22 +0800 Subject: [PATCH 3/5] improve arrays check equals or not --- .../java/org/apache/bookkeeper/bookie/FileSystemUpgrade.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileSystemUpgrade.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileSystemUpgrade.java index f60467b1f67..03cbe9f392a 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileSystemUpgrade.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileSystemUpgrade.java @@ -31,6 +31,7 @@ import java.io.IOException; import java.net.MalformedURLException; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -101,7 +102,8 @@ private static List getAllDirectories(ServerConfiguration conf) { dirs.addAll(Lists.newArrayList(conf.getJournalDirs())); final File[] ledgerDirs = conf.getLedgerDirs(); final File[] indexDirs = conf.getIndexDirs(); - if (indexDirs != null && indexDirs != ledgerDirs) { + if (indexDirs != null && + !Arrays.asList(indexDirs).equals(Arrays.asList(ledgerDirs))) { dirs.addAll(Lists.newArrayList(indexDirs)); } Collections.addAll(dirs, ledgerDirs); From 643e56933b29062bfc10212093fce3b004b1f7e2 Mon Sep 17 00:00:00 2001 From: wenbingshen Date: Fri, 3 Feb 2023 11:19:49 +0800 Subject: [PATCH 4/5] fix checkstyle --- .../java/org/apache/bookkeeper/bookie/FileSystemUpgrade.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileSystemUpgrade.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileSystemUpgrade.java index 03cbe9f392a..cef1bb82b7c 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileSystemUpgrade.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileSystemUpgrade.java @@ -102,8 +102,8 @@ private static List getAllDirectories(ServerConfiguration conf) { dirs.addAll(Lists.newArrayList(conf.getJournalDirs())); final File[] ledgerDirs = conf.getLedgerDirs(); final File[] indexDirs = conf.getIndexDirs(); - if (indexDirs != null && - !Arrays.asList(indexDirs).equals(Arrays.asList(ledgerDirs))) { + if (indexDirs != null + && !Arrays.asList(indexDirs).equals(Arrays.asList(ledgerDirs))) { dirs.addAll(Lists.newArrayList(indexDirs)); } Collections.addAll(dirs, ledgerDirs); From 2dcd69aa2b1a7dd4c1175a3a12e8efaebd376614 Mon Sep 17 00:00:00 2001 From: wenbingshen Date: Fri, 3 Feb 2023 15:28:11 +0800 Subject: [PATCH 5/5] fix check dirs equals function --- .../bookkeeper/bookie/FileSystemUpgrade.java | 8 ++-- .../apache/bookkeeper/bookie/UpgradeTest.java | 41 +++++++++++++++++++ 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileSystemUpgrade.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileSystemUpgrade.java index cef1bb82b7c..8fff510c562 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileSystemUpgrade.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileSystemUpgrade.java @@ -24,6 +24,7 @@ import static java.nio.charset.StandardCharsets.UTF_8; import static org.apache.bookkeeper.meta.MetadataDrivers.runFunctionWithRegistrationManager; +import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.Lists; import com.google.common.util.concurrent.UncheckedExecutionException; import java.io.File; @@ -97,13 +98,14 @@ public boolean accept(File dir, String name) { } }; - private static List getAllDirectories(ServerConfiguration conf) { + @VisibleForTesting + public static List getAllDirectories(ServerConfiguration conf) { List dirs = new ArrayList<>(); dirs.addAll(Lists.newArrayList(conf.getJournalDirs())); final File[] ledgerDirs = conf.getLedgerDirs(); final File[] indexDirs = conf.getIndexDirs(); - if (indexDirs != null - && !Arrays.asList(indexDirs).equals(Arrays.asList(ledgerDirs))) { + if (indexDirs != null && indexDirs.length == ledgerDirs.length + && !Arrays.asList(indexDirs).containsAll(Arrays.asList(ledgerDirs))) { dirs.addAll(Lists.newArrayList(indexDirs)); } Collections.addAll(dirs, ledgerDirs); diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/UpgradeTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/UpgradeTest.java index 8b7680785f3..e484f91133b 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/UpgradeTest.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/UpgradeTest.java @@ -21,6 +21,7 @@ package org.apache.bookkeeper.bookie; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -36,6 +37,7 @@ import java.nio.ByteBuffer; import java.nio.channels.FileChannel; import java.util.Arrays; +import java.util.List; import org.apache.bookkeeper.client.ClientUtil; import org.apache.bookkeeper.client.LedgerHandle; import org.apache.bookkeeper.conf.ServerConfiguration; @@ -356,4 +358,43 @@ public void testCommandLine() throws Exception { System.setErr(origerr); } } + + @Test + public void testFSUGetAllDirectories() throws Exception { + ServerConfiguration conf = TestBKConfiguration.newServerConfiguration(); + final File journalDir = tmpDirs.createNew("bookie", "journal"); + final File ledgerDir1 = tmpDirs.createNew("bookie", "ledger"); + final File ledgerDir2 = tmpDirs.createNew("bookie", "ledger"); + + // test1 + conf.setJournalDirName(journalDir.getPath()) + .setLedgerDirNames(new String[]{ledgerDir1.getPath(), ledgerDir2.getPath()}) + .setIndexDirName(new String[]{ledgerDir1.getPath(), ledgerDir2.getPath()}); + List allDirectories = FileSystemUpgrade.getAllDirectories(conf); + assertEquals(3, allDirectories.size()); + + // test2 + conf.setJournalDirName(journalDir.getPath()) + .setLedgerDirNames(new String[]{ledgerDir1.getPath(), ledgerDir2.getPath()}) + .setIndexDirName(new String[]{ledgerDir2.getPath(), ledgerDir1.getPath()}); + allDirectories = FileSystemUpgrade.getAllDirectories(conf); + assertEquals(3, allDirectories.size()); + + final File indexDir1 = tmpDirs.createNew("bookie", "index"); + final File indexDir2 = tmpDirs.createNew("bookie", "index"); + + // test3 + conf.setJournalDirName(journalDir.getPath()) + .setLedgerDirNames(new String[]{ledgerDir1.getPath(), ledgerDir2.getPath()}) + .setIndexDirName(new String[]{indexDir1.getPath(), indexDir2.getPath()}); + allDirectories = FileSystemUpgrade.getAllDirectories(conf); + assertEquals(5, allDirectories.size()); + + // test4 + conf.setJournalDirName(journalDir.getPath()) + .setLedgerDirNames(new String[]{ledgerDir1.getPath(), ledgerDir2.getPath()}) + .setIndexDirName(new String[]{indexDir2.getPath(), indexDir1.getPath()}); + allDirectories = FileSystemUpgrade.getAllDirectories(conf); + assertEquals(5, allDirectories.size()); + } }