From 92aa2f1e62c363e6521d9fdf7080ce0807071527 Mon Sep 17 00:00:00 2001 From: horizonzy Date: Mon, 25 Sep 2023 13:29:26 +0800 Subject: [PATCH 1/5] Make after version5 cookie index dir compatible with before version5. --- .../java/org/apache/bookkeeper/bookie/Cookie.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Cookie.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Cookie.java index d7502a54b69..4c16a2eca82 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Cookie.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Cookie.java @@ -138,14 +138,14 @@ private boolean verifyLedgerDirs(Cookie c, boolean checkIfSuperSet) { } private boolean verifyIndexDirs(Cookie c, boolean checkIfSuperSet) { - // compatible logic: existed node's cookie has no indexDirs, the indexDirs's default value is ledgerDirs. - String indexDirsInConfig = StringUtils.isNotBlank(indexDirs) ? indexDirs : ledgerDirs; - String indexDirsInCookie = StringUtils.isNotBlank(c.indexDirs) ? c.indexDirs : c.ledgerDirs; - + // compatible logic: version 5 introduce `indexDirs`, we shouldn't check `indexDirs` with the before version. + if (this.layoutVersion >= 5 && c.layoutVersion < 5) { + return true; + } if (!checkIfSuperSet) { - return indexDirsInConfig.equals(indexDirsInCookie); + return indexDirs.equals(c.indexDirs); } else { - return isSuperSet(decodeDirPathFromCookie(indexDirsInConfig), decodeDirPathFromCookie(indexDirsInCookie)); + return isSuperSet(decodeDirPathFromCookie(indexDirs), decodeDirPathFromCookie(c.indexDirs)); } } @@ -194,7 +194,7 @@ public String toString() { } StringBuilder b = new StringBuilder(); - b.append(CURRENT_COOKIE_LAYOUT_VERSION).append("\n"); + b.append(layoutVersion).append("\n"); b.append(builder.build()); return b.toString(); } From b1f2c486de5869fefd30ab8f458a5a895b63fcef Mon Sep 17 00:00:00 2001 From: horizonzy Date: Mon, 25 Sep 2023 15:33:50 +0800 Subject: [PATCH 2/5] Fix ci. --- .../src/main/java/org/apache/bookkeeper/bookie/Cookie.java | 1 - 1 file changed, 1 deletion(-) diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Cookie.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Cookie.java index 4c16a2eca82..11d44bcb708 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Cookie.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Cookie.java @@ -50,7 +50,6 @@ import org.apache.bookkeeper.versioning.LongVersion; import org.apache.bookkeeper.versioning.Version; import org.apache.bookkeeper.versioning.Versioned; -import org.apache.commons.lang3.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; From 4bee6a050f680ee13c600951fe7e57668361861a Mon Sep 17 00:00:00 2001 From: horizonzy Date: Tue, 26 Sep 2023 09:44:20 +0800 Subject: [PATCH 3/5] Fix ci. --- .../src/main/java/org/apache/bookkeeper/bookie/Cookie.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Cookie.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Cookie.java index 11d44bcb708..f04ce186674 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Cookie.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Cookie.java @@ -141,6 +141,12 @@ private boolean verifyIndexDirs(Cookie c, boolean checkIfSuperSet) { if (this.layoutVersion >= 5 && c.layoutVersion < 5) { return true; } + if (indexDirs == null && c.indexDirs == null) { + return true; + } + if (indexDirs == null || c.indexDirs == null) { + return false; + } if (!checkIfSuperSet) { return indexDirs.equals(c.indexDirs); } else { From 953faae26a8fc908500dea5fbc924a8f60813faf Mon Sep 17 00:00:00 2001 From: horizonzy Date: Tue, 26 Sep 2023 12:19:33 +0800 Subject: [PATCH 4/5] Tune test case. --- .../bookie/LegacyCookieValidation.java | 4 +- .../bookkeeper/bookie/CookieIndexDirTest.java | 67 +++++++++++++++---- 2 files changed, 56 insertions(+), 15 deletions(-) diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LegacyCookieValidation.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LegacyCookieValidation.java index 2a9744d6820..aeed232b51c 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LegacyCookieValidation.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LegacyCookieValidation.java @@ -17,6 +17,7 @@ */ package org.apache.bookkeeper.bookie; +import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.Lists; import java.io.File; import java.io.FileNotFoundException; @@ -221,7 +222,8 @@ private static void verifyDirsForNewEnvironment(List missedCookieDirs) } } - private static void stampNewCookie(ServerConfiguration conf, + @VisibleForTesting + public static void stampNewCookie(ServerConfiguration conf, Cookie masterCookie, RegistrationManager rm, Version version, diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CookieIndexDirTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CookieIndexDirTest.java index 3caf076082e..ee426e74de7 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CookieIndexDirTest.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CookieIndexDirTest.java @@ -39,6 +39,8 @@ import java.util.List; import java.util.Random; import java.util.Set; +import java.util.UUID; + import org.apache.bookkeeper.bookie.BookieException.InvalidCookieException; import org.apache.bookkeeper.client.BookKeeperAdmin; import org.apache.bookkeeper.conf.ServerConfiguration; @@ -974,31 +976,68 @@ public void testBookieIdSetting() throws Exception { /** * Compatibility test - * 1. First create bookie without indexDirName - * 2. Configure indexDirName to start bookie */ @Test - public void testNewBookieStartingWithOldCookie() throws Exception { + public void testCookieCompatibility() throws Exception { String journalDir = newDirectory(); String[] ledgerDirs = {newDirectory(), newDirectory()}; - ServerConfiguration conf = TestBKConfiguration.newServerConfiguration(); - conf.setJournalDirName(journalDir) + String[] indexDirs = {newDirectory()}; + + ServerConfiguration confWithDirs = TestBKConfiguration.newServerConfiguration(); + confWithDirs.setJournalDirName(journalDir) .setLedgerDirNames(ledgerDirs) + .setIndexDirName(indexDirs) .setBookiePort(bookiePort) .setMetadataServiceUri(zkUtil.getMetadataServiceUri()); - validateConfig(conf); - - conf = TestBKConfiguration.newServerConfiguration(); - conf.setJournalDirName(journalDir) + String instanceId = UUID.randomUUID().toString(); + + Cookie.Builder builder = Cookie.generateCookie(confWithDirs); + builder.setLayoutVersion(4); + builder.setIndexDirs(null); + builder.setInstanceId(instanceId); + Cookie version4Cookie = builder.build(); + + Cookie.Builder builder1 = Cookie.generateCookie(confWithDirs); + builder1.setInstanceId(instanceId); + Cookie version5CookieWithIndexDirs = builder1.build(); + + ServerConfiguration confWithoutDirs = TestBKConfiguration.newServerConfiguration(); + confWithoutDirs.setJournalDirName(journalDir) .setLedgerDirNames(ledgerDirs) - .setIndexDirName(ledgerDirs) .setBookiePort(bookiePort) .setMetadataServiceUri(zkUtil.getMetadataServiceUri()); + Cookie.Builder builder2 = Cookie.generateCookie(confWithoutDirs); + builder2.setInstanceId(instanceId); + Cookie version5CookieWithoutIndexDirs = builder2.build(); + + version5CookieWithIndexDirs.verify(version4Cookie); + version5CookieWithIndexDirs.verifyIsSuperSet(version4Cookie); + + version5CookieWithoutIndexDirs.verify(version4Cookie); + version5CookieWithoutIndexDirs.verifyIsSuperSet(version4Cookie); + try { - validateConfig(conf); - } catch (InvalidCookieException ice) { - // error behaviour - fail("Validate failed, error info: " + ice.getMessage()); + version5CookieWithIndexDirs.verify(version5CookieWithoutIndexDirs); + fail("Shouldn't compatible"); + } catch (Exception ignore) { + } + + try { + version5CookieWithIndexDirs.verifyIsSuperSet(version5CookieWithoutIndexDirs); + fail("Shouldn't compatible"); + } catch (Exception ignore) { + } + + try { + version5CookieWithoutIndexDirs.verify(version5CookieWithIndexDirs); + fail("Shouldn't compatible"); + } catch (Exception ignore) { + } + + try { + version5CookieWithoutIndexDirs.verifyIsSuperSet(version5CookieWithIndexDirs); + fail("Shouldn't compatible"); + } catch (Exception ignore) { } } } From 18b1b028bd35ec46a5f3420014b61f38141d02a3 Mon Sep 17 00:00:00 2001 From: horizonzy Date: Tue, 26 Sep 2023 12:29:28 +0800 Subject: [PATCH 5/5] Fix ci. --- .../bookkeeper/bookie/CookieIndexDirTest.java | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CookieIndexDirTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CookieIndexDirTest.java index ee426e74de7..d5bb60a9a78 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CookieIndexDirTest.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CookieIndexDirTest.java @@ -40,7 +40,6 @@ import java.util.Random; import java.util.Set; import java.util.UUID; - import org.apache.bookkeeper.bookie.BookieException.InvalidCookieException; import org.apache.bookkeeper.client.BookKeeperAdmin; import org.apache.bookkeeper.conf.ServerConfiguration; @@ -975,14 +974,14 @@ public void testBookieIdSetting() throws Exception { } /** - * Compatibility test + * Compatibility test. */ @Test public void testCookieCompatibility() throws Exception { String journalDir = newDirectory(); String[] ledgerDirs = {newDirectory(), newDirectory()}; String[] indexDirs = {newDirectory()}; - + ServerConfiguration confWithDirs = TestBKConfiguration.newServerConfiguration(); confWithDirs.setJournalDirName(journalDir) .setLedgerDirNames(ledgerDirs) @@ -990,17 +989,17 @@ public void testCookieCompatibility() throws Exception { .setBookiePort(bookiePort) .setMetadataServiceUri(zkUtil.getMetadataServiceUri()); String instanceId = UUID.randomUUID().toString(); - + Cookie.Builder builder = Cookie.generateCookie(confWithDirs); builder.setLayoutVersion(4); builder.setIndexDirs(null); builder.setInstanceId(instanceId); Cookie version4Cookie = builder.build(); - + Cookie.Builder builder1 = Cookie.generateCookie(confWithDirs); builder1.setInstanceId(instanceId); Cookie version5CookieWithIndexDirs = builder1.build(); - + ServerConfiguration confWithoutDirs = TestBKConfiguration.newServerConfiguration(); confWithoutDirs.setJournalDirName(journalDir) .setLedgerDirNames(ledgerDirs) @@ -1009,31 +1008,31 @@ public void testCookieCompatibility() throws Exception { Cookie.Builder builder2 = Cookie.generateCookie(confWithoutDirs); builder2.setInstanceId(instanceId); Cookie version5CookieWithoutIndexDirs = builder2.build(); - + version5CookieWithIndexDirs.verify(version4Cookie); version5CookieWithIndexDirs.verifyIsSuperSet(version4Cookie); - + version5CookieWithoutIndexDirs.verify(version4Cookie); version5CookieWithoutIndexDirs.verifyIsSuperSet(version4Cookie); - + try { version5CookieWithIndexDirs.verify(version5CookieWithoutIndexDirs); fail("Shouldn't compatible"); } catch (Exception ignore) { } - + try { version5CookieWithIndexDirs.verifyIsSuperSet(version5CookieWithoutIndexDirs); fail("Shouldn't compatible"); } catch (Exception ignore) { } - + try { version5CookieWithoutIndexDirs.verify(version5CookieWithIndexDirs); fail("Shouldn't compatible"); } catch (Exception ignore) { } - + try { version5CookieWithoutIndexDirs.verifyIsSuperSet(version5CookieWithIndexDirs); fail("Shouldn't compatible");