You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@bookkeeper.apache.org by GitBox <gi...@apache.org> on 2021/08/24 12:57:34 UTC
[GitHub] [bookkeeper] Vanlightly opened a new pull request #2774: ISSUE-2773: Add db ledgers index rebuild op
Vanlightly opened a new pull request #2774:
URL: https://github.com/apache/bookkeeper/pull/2774
### Motivation
File corruption of the ledgers index (DbLedgerStorage) has happened in production and we currently have no good way of resolving it. There exists a locations index rebuild command, this issue describes a new command targeted at the ledgers index.
This command should only be run when either the bookie is offline, or when in readonly mode to avoid data loss. Container based environments may not be able to make the bookie go offline while also allowing the shell or bkctl to perform the operation.
The db ledgers index rebuild op does the following:
moves the current index file to a backup location
scans all journal and entry log files to discover all ledger ids currently stored.
builds a new index where each ledger is fenced and has an empty master key set.
The bookie should then be restarted again (in normal mode) to load the rebuilt index
If a failure occurs during the operation, the original index file is restored to its original location.
Notable stuff:
The reason for setting fencing each ledger is that there is no safe way of setting the fenced status based on metadata while the bookie is running. If the command is run when the bookie is in readonly mode, it can still serve fence requests and any fencing that occurs while the rebuild is occurring will be lost, which breaks the data safety guarantees of the BookKeeper protocol. Given also that the bookie should at least be in readonly mode (else offline), it is likely a member of the current ensemble of few ledgers so the impact should be minimal.
The reason for setting an empty master key is that firstly, an empty master key simply gets overwritten, so cannot cause an IllegalOpException. Secondly, if we use the password stored in metadata, then we need to be sure to use exactly the same digest algorithm as the client when it creates the ledgerKey, else the bookie will start failing all writes. This could potentially cause a problem in the future if the way the ledgerKey is generated changes (old clients would be incompatible after a ledgers index rebuild).
### Changes
Adds two new commands. "rebuild-db-ledgers-index" rebuilds the db ledgers index by scanning the journal and entry log files. "check-db-ledgers-index" performs a read scan of the db ledgers index, this can be used when corruption is suspected.
Additionally:
- a test of the new rebuild op
- makes the Journal. listJournalIds() method public so that the command can iterate over the journal files.
- rewords some printed statements in the LocationsIndexRebuildOp to make it clear which index it is rebuilding (given that there are two indexes in the DbLedgerStorage).
--
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: issues-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] hsaputra commented on pull request #2774: ISSUE-2773: Add db ledgers index rebuild op
Posted by GitBox <gi...@apache.org>.
hsaputra commented on pull request #2774:
URL: https://github.com/apache/bookkeeper/pull/2774#issuecomment-926052670
Will merge this EOD PST if no more comments. Thanks
--
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: issues-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] eolivelli commented on a change in pull request #2774: ISSUE-2773: Add db ledgers index rebuild op
Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #2774:
URL: https://github.com/apache/bookkeeper/pull/2774#discussion_r714233152
##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/LedgersIndexCheckOp.java
##########
@@ -0,0 +1,102 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+package org.apache.bookkeeper.bookie.storage.ldb;
+
+import java.io.IOException;
+import java.nio.file.FileSystems;
+import java.nio.file.Path;
+import java.util.Base64;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+import org.apache.bookkeeper.bookie.BookieImpl;
+import org.apache.bookkeeper.bookie.storage.ldb.KeyValueStorageFactory.DbConfigType;
+import org.apache.bookkeeper.conf.ServerConfiguration;
+import org.apache.commons.lang.time.DurationFormatUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Scan the ledgers index to make sure it is readable.
+ */
+public class LedgersIndexCheckOp {
+ private static final Logger LOG = LoggerFactory.getLogger(LedgersIndexCheckOp.class);
+
+ private final ServerConfiguration conf;
+ private final boolean verbose;
+ private static final String LedgersSubPath = "ledgers";
+
+ public LedgersIndexCheckOp(ServerConfiguration conf, boolean verbose) {
+ this.conf = conf;
+ this.verbose = verbose;
+ }
+
+ public boolean initiate() throws IOException {
+ String basePath = BookieImpl.getCurrentDirectory(conf.getLedgerDirs()[0]).toString();
+ Path currentPath = FileSystems.getDefault().getPath(basePath, LedgersSubPath);
+
+ LOG.info("Loading ledgers index from {}", currentPath);
+
+ long startTime = System.nanoTime();
+ LOG.info("Starting index scan");
+
+ try {
+ KeyValueStorage index = new KeyValueStorageRocksDB(basePath, LedgersSubPath,
+ DbConfigType.Small, conf, true);
+ // Read all ledgers from db
+ KeyValueStorage.CloseableIterator<Map.Entry<byte[], byte[]>> iterator = index.iterator();
+ int ctr = 0;
+ try {
+ while (iterator.hasNext()) {
+ ctr++;
+ Map.Entry<byte[], byte[]> entry = iterator.next();
+ long ledgerId = ArrayUtil.getLong(entry.getKey(), 0);
+ DbLedgerStorageDataFormats.LedgerData ledgerData =
+ DbLedgerStorageDataFormats.LedgerData.parseFrom(entry.getValue());
+ if (verbose) {
+ LOG.info("Ledger {}, exists: {}, isFenced: {}, masterKey: {}, explicitLAC: {}",
+ ledgerId,
+ (ledgerData.hasExists() ? ledgerData.getExists() : "-"),
+ (ledgerData.hasFenced() ? ledgerData.getFenced() : "-"),
+ (ledgerData.hasMasterKey()
+ ? Base64.getEncoder()
+ .encodeToString(ledgerData.getMasterKey().toByteArray())
+ : "-"),
+ (ledgerData.hasExplicitLac() ? ledgerData.getExplicitLac() : "-"));
+ } else if (ctr % 100 == 0) {
Review comment:
Maybe printing the number of ledgers is useful in verbose mode as well
--
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: issues-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] hsaputra commented on pull request #2774: ISSUE-2773: Add db ledgers index rebuild op
Posted by GitBox <gi...@apache.org>.
hsaputra commented on pull request #2774:
URL: https://github.com/apache/bookkeeper/pull/2774#issuecomment-925994953
rerun failure checks
--
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: issues-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] ivankelly commented on a change in pull request #2774: ISSUE-2773: Add db ledgers index rebuild op
Posted by GitBox <gi...@apache.org>.
ivankelly commented on a change in pull request #2774:
URL: https://github.com/apache/bookkeeper/pull/2774#discussion_r695158532
##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/LedgersIndexRebuildOp.java
##########
@@ -0,0 +1,214 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+package org.apache.bookkeeper.bookie.storage.ldb;
+
+import com.google.common.collect.Lists;
+import com.google.protobuf.ByteString;
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.Unpooled;
+import java.io.File;
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.file.FileSystems;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.text.SimpleDateFormat;
+import java.util.Date;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import org.apache.bookkeeper.bookie.BookieImpl;
+import org.apache.bookkeeper.bookie.EntryLogger;
+import org.apache.bookkeeper.bookie.EntryLogger.EntryLogScanner;
+import org.apache.bookkeeper.bookie.Journal;
+import org.apache.bookkeeper.bookie.LedgerDirsManager;
+import org.apache.bookkeeper.bookie.storage.ldb.KeyValueStorageFactory.DbConfigType;
+import org.apache.bookkeeper.conf.ServerConfiguration;
+import org.apache.bookkeeper.util.BookKeeperConstants;
+import org.apache.bookkeeper.util.DiskChecker;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Scan all entries in the journal and entry log files then rebuilds the ledgers index.
+ * Notable stuff:
+ * - Fences every ledger as even if we check the metadata, we cannot guarantee that
+ * a fence request was served while the rebuild was taking place (even if the bookie
+ * is running in read-only mode).
+ * Losing the fenced status of a ledger is UNSAFE.
+ * - Sets the master key as an empty byte array. This is correct as empty master keys
+ * are overwritten and we cannot use the password from metadata, and cannot know 100%
+ * for sure how a digest for the password was generated.
+ */
+public class LedgersIndexRebuildOp {
+ private static final Logger LOG = LoggerFactory.getLogger(LedgersIndexRebuildOp.class);
+
+ private final ServerConfiguration conf;
+ private final boolean verbose;
+ private static final String LedgersSubPath = "ledgers";
+
+ public LedgersIndexRebuildOp(ServerConfiguration conf, boolean verbose) {
+ this.conf = conf;
+ this.verbose = verbose;
+ }
+
+ public boolean initiate() {
+ LOG.info("Starting ledger index rebuilding");
+
+ // Move locations index to a backup directory
+ String basePath = BookieImpl.getCurrentDirectory(conf.getLedgerDirs()[0]).toString();
+ Path currentPath = FileSystems.getDefault().getPath(basePath, LedgersSubPath);
+ String timestamp = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSSZ").format(new Date());
+ Path backupPath = FileSystems.getDefault().getPath(basePath, LedgersSubPath + ".BACKUP-" + timestamp);
+ Path abortPath = FileSystems.getDefault().getPath(basePath, LedgersSubPath + ".ABORT-" + timestamp);
+ try {
+ Files.move(currentPath, backupPath);
+ } catch (IOException e) {
+ LOG.error("Could not move index to a backup location", e);
+ return false;
+ }
+
+ LOG.info("Created ledgers index backup at {}", backupPath);
+ LOG.info("Starting scan phase (scans journal and entry log files)");
+
+ try {
+ Set<Long> ledgers = new HashSet<>();
+ scanJournals(ledgers);
+ scanEntryLogFiles(ledgers);
+
+ LOG.info("Scan complete, found {} ledgers. "
+ + "Starting to build a new ledgers index", ledgers.size());
+
+ KeyValueStorage newIndex = KeyValueStorageRocksDB.factory.newKeyValueStorage(
+ basePath, LedgersSubPath, DbConfigType.Small, conf);
+
+ for (Long ledgerId : ledgers) {
+ DbLedgerStorageDataFormats.LedgerData ledgerData =
+ DbLedgerStorageDataFormats.LedgerData.newBuilder()
+ .setExists(true)
+ .setFenced(true)
+ .setMasterKey(ByteString.EMPTY).build();
+
+ byte[] ledgerArray = new byte[16];
+ ArrayUtil.setLong(ledgerArray, 0, ledgerId);
+ newIndex.put(ledgerArray, ledgerData.toByteArray());
+ }
+
+ newIndex.sync();
+ newIndex.close();
+ } catch (Throwable t) {
+ LOG.error("Error during rebuild", t);
+ restoreBackup(currentPath, backupPath, abortPath);
Review comment:
why even move the original at the start? it creates a lot of surface to fail, and then you have to unwind. Instead you could create the new index, and if successful, move old and move in new, which have fewer ops that can fail.
--
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: issues-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] Vanlightly commented on a change in pull request #2774: ISSUE-2773: Add db ledgers index rebuild op
Posted by GitBox <gi...@apache.org>.
Vanlightly commented on a change in pull request #2774:
URL: https://github.com/apache/bookkeeper/pull/2774#discussion_r714943728
##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/LedgersIndexRebuildOp.java
##########
@@ -0,0 +1,214 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+package org.apache.bookkeeper.bookie.storage.ldb;
+
+import com.google.common.collect.Lists;
+import com.google.protobuf.ByteString;
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.Unpooled;
+import java.io.File;
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.file.FileSystems;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.text.SimpleDateFormat;
+import java.util.Date;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import org.apache.bookkeeper.bookie.BookieImpl;
+import org.apache.bookkeeper.bookie.EntryLogger;
+import org.apache.bookkeeper.bookie.EntryLogger.EntryLogScanner;
+import org.apache.bookkeeper.bookie.Journal;
+import org.apache.bookkeeper.bookie.LedgerDirsManager;
+import org.apache.bookkeeper.bookie.storage.ldb.KeyValueStorageFactory.DbConfigType;
+import org.apache.bookkeeper.conf.ServerConfiguration;
+import org.apache.bookkeeper.util.BookKeeperConstants;
+import org.apache.bookkeeper.util.DiskChecker;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Scan all entries in the journal and entry log files then rebuilds the ledgers index.
+ * Notable stuff:
+ * - Fences every ledger as even if we check the metadata, we cannot guarantee that
+ * a fence request was served while the rebuild was taking place (even if the bookie
+ * is running in read-only mode).
+ * Losing the fenced status of a ledger is UNSAFE.
+ * - Sets the master key as an empty byte array. This is correct as empty master keys
+ * are overwritten and we cannot use the password from metadata, and cannot know 100%
+ * for sure how a digest for the password was generated.
+ */
+public class LedgersIndexRebuildOp {
+ private static final Logger LOG = LoggerFactory.getLogger(LedgersIndexRebuildOp.class);
+
+ private final ServerConfiguration conf;
+ private final boolean verbose;
+ private static final String LedgersSubPath = "ledgers";
+
+ public LedgersIndexRebuildOp(ServerConfiguration conf, boolean verbose) {
+ this.conf = conf;
+ this.verbose = verbose;
+ }
+
+ public boolean initiate() {
+ LOG.info("Starting ledger index rebuilding");
+
+ String timestamp = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSSZ").format(new Date());
+ String basePath = BookieImpl.getCurrentDirectory(conf.getLedgerDirs()[0]).toString();
+ String tempLedgersSubPath = LedgersSubPath + ".TEMP-" + timestamp;
+ Path tempPath = FileSystems.getDefault().getPath(basePath, tempLedgersSubPath);
+ Path currentPath = FileSystems.getDefault().getPath(basePath, LedgersSubPath);
+
+ LOG.info("Starting scan phase (scans journal and entry log files)");
+
+ try {
+ Set<Long> ledgers = new HashSet<>();
+ scanJournals(ledgers);
+ scanEntryLogFiles(ledgers);
+
+ LOG.info("Scan complete, found {} ledgers. "
+ + "Starting to build a new ledgers index", ledgers.size());
+
+ KeyValueStorage newIndex = KeyValueStorageRocksDB.factory.newKeyValueStorage(
+ basePath, tempLedgersSubPath, DbConfigType.Small, conf);
+ LOG.info("Created ledgers index at temp location {}", tempPath);
+
+ for (Long ledgerId : ledgers) {
+ DbLedgerStorageDataFormats.LedgerData ledgerData =
+ DbLedgerStorageDataFormats.LedgerData.newBuilder()
+ .setExists(true)
+ .setFenced(true)
+ .setMasterKey(ByteString.EMPTY).build();
+
+ byte[] ledgerArray = new byte[16];
+ ArrayUtil.setLong(ledgerArray, 0, ledgerId);
+ newIndex.put(ledgerArray, ledgerData.toByteArray());
+ }
+
+ newIndex.sync();
+ newIndex.close();
+ } catch (Throwable t) {
+ LOG.error("Error during rebuild, the original index remains unchanged", t);
+ delete(tempPath);
+ return false;
+ }
+
+ // replace the existing index
+ try {
+ Path prevPath = FileSystems.getDefault().getPath(basePath, LedgersSubPath + ".PREV-" + timestamp);
+ Files.move(currentPath, prevPath);
+ Files.move(tempPath, currentPath);
Review comment:
If the first move fails, then the index should be in its original or back-up location. If the second move fails, the rebuilt index will still be in its temp location. Rather than try to auto fix after an error here, I'll add better logging to help the administrator fix it themselves. Seems like a highly unlikely scenario for a move to fail.
--
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: issues-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] Vanlightly commented on a change in pull request #2774: ISSUE-2773: Add db ledgers index rebuild op
Posted by GitBox <gi...@apache.org>.
Vanlightly commented on a change in pull request #2774:
URL: https://github.com/apache/bookkeeper/pull/2774#discussion_r714950461
##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/LedgersIndexRebuildOp.java
##########
@@ -0,0 +1,214 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+package org.apache.bookkeeper.bookie.storage.ldb;
+
+import com.google.common.collect.Lists;
+import com.google.protobuf.ByteString;
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.Unpooled;
+import java.io.File;
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.file.FileSystems;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.text.SimpleDateFormat;
+import java.util.Date;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import org.apache.bookkeeper.bookie.BookieImpl;
+import org.apache.bookkeeper.bookie.EntryLogger;
+import org.apache.bookkeeper.bookie.EntryLogger.EntryLogScanner;
+import org.apache.bookkeeper.bookie.Journal;
+import org.apache.bookkeeper.bookie.LedgerDirsManager;
+import org.apache.bookkeeper.bookie.storage.ldb.KeyValueStorageFactory.DbConfigType;
+import org.apache.bookkeeper.conf.ServerConfiguration;
+import org.apache.bookkeeper.util.BookKeeperConstants;
+import org.apache.bookkeeper.util.DiskChecker;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Scan all entries in the journal and entry log files then rebuilds the ledgers index.
+ * Notable stuff:
+ * - Fences every ledger as even if we check the metadata, we cannot guarantee that
+ * a fence request was served while the rebuild was taking place (even if the bookie
+ * is running in read-only mode).
+ * Losing the fenced status of a ledger is UNSAFE.
+ * - Sets the master key as an empty byte array. This is correct as empty master keys
+ * are overwritten and we cannot use the password from metadata, and cannot know 100%
+ * for sure how a digest for the password was generated.
+ */
+public class LedgersIndexRebuildOp {
+ private static final Logger LOG = LoggerFactory.getLogger(LedgersIndexRebuildOp.class);
+
+ private final ServerConfiguration conf;
+ private final boolean verbose;
+ private static final String LedgersSubPath = "ledgers";
+
+ public LedgersIndexRebuildOp(ServerConfiguration conf, boolean verbose) {
+ this.conf = conf;
+ this.verbose = verbose;
+ }
+
+ public boolean initiate() {
+ LOG.info("Starting ledger index rebuilding");
+
+ String timestamp = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSSZ").format(new Date());
+ String basePath = BookieImpl.getCurrentDirectory(conf.getLedgerDirs()[0]).toString();
+ String tempLedgersSubPath = LedgersSubPath + ".TEMP-" + timestamp;
+ Path tempPath = FileSystems.getDefault().getPath(basePath, tempLedgersSubPath);
+ Path currentPath = FileSystems.getDefault().getPath(basePath, LedgersSubPath);
+
+ LOG.info("Starting scan phase (scans journal and entry log files)");
+
+ try {
+ Set<Long> ledgers = new HashSet<>();
+ scanJournals(ledgers);
+ scanEntryLogFiles(ledgers);
+
+ LOG.info("Scan complete, found {} ledgers. "
+ + "Starting to build a new ledgers index", ledgers.size());
+
+ KeyValueStorage newIndex = KeyValueStorageRocksDB.factory.newKeyValueStorage(
+ basePath, tempLedgersSubPath, DbConfigType.Small, conf);
+ LOG.info("Created ledgers index at temp location {}", tempPath);
+
+ for (Long ledgerId : ledgers) {
+ DbLedgerStorageDataFormats.LedgerData ledgerData =
+ DbLedgerStorageDataFormats.LedgerData.newBuilder()
+ .setExists(true)
+ .setFenced(true)
+ .setMasterKey(ByteString.EMPTY).build();
+
+ byte[] ledgerArray = new byte[16];
+ ArrayUtil.setLong(ledgerArray, 0, ledgerId);
+ newIndex.put(ledgerArray, ledgerData.toByteArray());
+ }
+
+ newIndex.sync();
+ newIndex.close();
+ } catch (Throwable t) {
+ LOG.error("Error during rebuild, the original index remains unchanged", t);
+ delete(tempPath);
+ return false;
+ }
+
+ // replace the existing index
+ try {
+ Path prevPath = FileSystems.getDefault().getPath(basePath, LedgersSubPath + ".PREV-" + timestamp);
+ Files.move(currentPath, prevPath);
+ Files.move(tempPath, currentPath);
+ LOG.info("Original index has been replaced with the new index. "
+ + "The original index has been moved to {}", prevPath);
+ } catch (IOException e) {
+ LOG.error("Could not replace the existing index", e);
Review comment:
I'll let the admin decide what to do here. I have added better logging of the move ops to help a human decide how to fix it.
--
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: issues-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] Vanlightly commented on pull request #2774: ISSUE-2773: Add db ledgers index rebuild op
Posted by GitBox <gi...@apache.org>.
Vanlightly commented on pull request #2774:
URL: https://github.com/apache/bookkeeper/pull/2774#issuecomment-916698839
@ivankelly @eolivelli any chance of a review?
--
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: issues-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] hsaputra merged pull request #2774: ISSUE-2773: Add db ledgers index rebuild op
Posted by GitBox <gi...@apache.org>.
hsaputra merged pull request #2774:
URL: https://github.com/apache/bookkeeper/pull/2774
--
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: issues-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] nicoloboschi commented on a change in pull request #2774: ISSUE-2773: Add db ledgers index rebuild op
Posted by GitBox <gi...@apache.org>.
nicoloboschi commented on a change in pull request #2774:
URL: https://github.com/apache/bookkeeper/pull/2774#discussion_r714556202
##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/LedgersIndexRebuildOp.java
##########
@@ -0,0 +1,214 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+package org.apache.bookkeeper.bookie.storage.ldb;
+
+import com.google.common.collect.Lists;
+import com.google.protobuf.ByteString;
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.Unpooled;
+import java.io.File;
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.file.FileSystems;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.text.SimpleDateFormat;
+import java.util.Date;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import org.apache.bookkeeper.bookie.BookieImpl;
+import org.apache.bookkeeper.bookie.EntryLogger;
+import org.apache.bookkeeper.bookie.EntryLogger.EntryLogScanner;
+import org.apache.bookkeeper.bookie.Journal;
+import org.apache.bookkeeper.bookie.LedgerDirsManager;
+import org.apache.bookkeeper.bookie.storage.ldb.KeyValueStorageFactory.DbConfigType;
+import org.apache.bookkeeper.conf.ServerConfiguration;
+import org.apache.bookkeeper.util.BookKeeperConstants;
+import org.apache.bookkeeper.util.DiskChecker;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Scan all entries in the journal and entry log files then rebuilds the ledgers index.
+ * Notable stuff:
+ * - Fences every ledger as even if we check the metadata, we cannot guarantee that
+ * a fence request was served while the rebuild was taking place (even if the bookie
+ * is running in read-only mode).
+ * Losing the fenced status of a ledger is UNSAFE.
+ * - Sets the master key as an empty byte array. This is correct as empty master keys
+ * are overwritten and we cannot use the password from metadata, and cannot know 100%
+ * for sure how a digest for the password was generated.
+ */
+public class LedgersIndexRebuildOp {
+ private static final Logger LOG = LoggerFactory.getLogger(LedgersIndexRebuildOp.class);
+
+ private final ServerConfiguration conf;
+ private final boolean verbose;
+ private static final String LedgersSubPath = "ledgers";
+
+ public LedgersIndexRebuildOp(ServerConfiguration conf, boolean verbose) {
+ this.conf = conf;
+ this.verbose = verbose;
+ }
+
+ public boolean initiate() {
+ LOG.info("Starting ledger index rebuilding");
+
+ String timestamp = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSSZ").format(new Date());
+ String basePath = BookieImpl.getCurrentDirectory(conf.getLedgerDirs()[0]).toString();
+ String tempLedgersSubPath = LedgersSubPath + ".TEMP-" + timestamp;
+ Path tempPath = FileSystems.getDefault().getPath(basePath, tempLedgersSubPath);
+ Path currentPath = FileSystems.getDefault().getPath(basePath, LedgersSubPath);
+
+ LOG.info("Starting scan phase (scans journal and entry log files)");
+
+ try {
+ Set<Long> ledgers = new HashSet<>();
+ scanJournals(ledgers);
+ scanEntryLogFiles(ledgers);
+
+ LOG.info("Scan complete, found {} ledgers. "
+ + "Starting to build a new ledgers index", ledgers.size());
+
+ KeyValueStorage newIndex = KeyValueStorageRocksDB.factory.newKeyValueStorage(
+ basePath, tempLedgersSubPath, DbConfigType.Small, conf);
+ LOG.info("Created ledgers index at temp location {}", tempPath);
+
+ for (Long ledgerId : ledgers) {
+ DbLedgerStorageDataFormats.LedgerData ledgerData =
+ DbLedgerStorageDataFormats.LedgerData.newBuilder()
+ .setExists(true)
+ .setFenced(true)
+ .setMasterKey(ByteString.EMPTY).build();
+
+ byte[] ledgerArray = new byte[16];
+ ArrayUtil.setLong(ledgerArray, 0, ledgerId);
+ newIndex.put(ledgerArray, ledgerData.toByteArray());
+ }
+
+ newIndex.sync();
+ newIndex.close();
+ } catch (Throwable t) {
+ LOG.error("Error during rebuild, the original index remains unchanged", t);
+ delete(tempPath);
+ return false;
+ }
+
+ // replace the existing index
+ try {
+ Path prevPath = FileSystems.getDefault().getPath(basePath, LedgersSubPath + ".PREV-" + timestamp);
+ Files.move(currentPath, prevPath);
+ Files.move(tempPath, currentPath);
+ LOG.info("Original index has been replaced with the new index. "
+ + "The original index has been moved to {}", prevPath);
+ } catch (IOException e) {
+ LOG.error("Could not replace the existing index", e);
Review comment:
drop temporary index?
##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/LedgersIndexRebuildOp.java
##########
@@ -0,0 +1,214 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+package org.apache.bookkeeper.bookie.storage.ldb;
+
+import com.google.common.collect.Lists;
+import com.google.protobuf.ByteString;
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.Unpooled;
+import java.io.File;
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.file.FileSystems;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.text.SimpleDateFormat;
+import java.util.Date;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import org.apache.bookkeeper.bookie.BookieImpl;
+import org.apache.bookkeeper.bookie.EntryLogger;
+import org.apache.bookkeeper.bookie.EntryLogger.EntryLogScanner;
+import org.apache.bookkeeper.bookie.Journal;
+import org.apache.bookkeeper.bookie.LedgerDirsManager;
+import org.apache.bookkeeper.bookie.storage.ldb.KeyValueStorageFactory.DbConfigType;
+import org.apache.bookkeeper.conf.ServerConfiguration;
+import org.apache.bookkeeper.util.BookKeeperConstants;
+import org.apache.bookkeeper.util.DiskChecker;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Scan all entries in the journal and entry log files then rebuilds the ledgers index.
+ * Notable stuff:
+ * - Fences every ledger as even if we check the metadata, we cannot guarantee that
+ * a fence request was served while the rebuild was taking place (even if the bookie
+ * is running in read-only mode).
+ * Losing the fenced status of a ledger is UNSAFE.
+ * - Sets the master key as an empty byte array. This is correct as empty master keys
+ * are overwritten and we cannot use the password from metadata, and cannot know 100%
+ * for sure how a digest for the password was generated.
+ */
+public class LedgersIndexRebuildOp {
+ private static final Logger LOG = LoggerFactory.getLogger(LedgersIndexRebuildOp.class);
+
+ private final ServerConfiguration conf;
+ private final boolean verbose;
+ private static final String LedgersSubPath = "ledgers";
+
+ public LedgersIndexRebuildOp(ServerConfiguration conf, boolean verbose) {
+ this.conf = conf;
+ this.verbose = verbose;
+ }
+
+ public boolean initiate() {
+ LOG.info("Starting ledger index rebuilding");
+
+ String timestamp = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSSZ").format(new Date());
+ String basePath = BookieImpl.getCurrentDirectory(conf.getLedgerDirs()[0]).toString();
+ String tempLedgersSubPath = LedgersSubPath + ".TEMP-" + timestamp;
+ Path tempPath = FileSystems.getDefault().getPath(basePath, tempLedgersSubPath);
+ Path currentPath = FileSystems.getDefault().getPath(basePath, LedgersSubPath);
+
+ LOG.info("Starting scan phase (scans journal and entry log files)");
+
+ try {
+ Set<Long> ledgers = new HashSet<>();
+ scanJournals(ledgers);
+ scanEntryLogFiles(ledgers);
+
+ LOG.info("Scan complete, found {} ledgers. "
+ + "Starting to build a new ledgers index", ledgers.size());
+
+ KeyValueStorage newIndex = KeyValueStorageRocksDB.factory.newKeyValueStorage(
Review comment:
try with resources ?
##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/LedgersIndexRebuildOp.java
##########
@@ -0,0 +1,214 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+package org.apache.bookkeeper.bookie.storage.ldb;
+
+import com.google.common.collect.Lists;
+import com.google.protobuf.ByteString;
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.Unpooled;
+import java.io.File;
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.file.FileSystems;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.text.SimpleDateFormat;
+import java.util.Date;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import org.apache.bookkeeper.bookie.BookieImpl;
+import org.apache.bookkeeper.bookie.EntryLogger;
+import org.apache.bookkeeper.bookie.EntryLogger.EntryLogScanner;
+import org.apache.bookkeeper.bookie.Journal;
+import org.apache.bookkeeper.bookie.LedgerDirsManager;
+import org.apache.bookkeeper.bookie.storage.ldb.KeyValueStorageFactory.DbConfigType;
+import org.apache.bookkeeper.conf.ServerConfiguration;
+import org.apache.bookkeeper.util.BookKeeperConstants;
+import org.apache.bookkeeper.util.DiskChecker;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Scan all entries in the journal and entry log files then rebuilds the ledgers index.
+ * Notable stuff:
+ * - Fences every ledger as even if we check the metadata, we cannot guarantee that
+ * a fence request was served while the rebuild was taking place (even if the bookie
+ * is running in read-only mode).
+ * Losing the fenced status of a ledger is UNSAFE.
+ * - Sets the master key as an empty byte array. This is correct as empty master keys
+ * are overwritten and we cannot use the password from metadata, and cannot know 100%
+ * for sure how a digest for the password was generated.
+ */
+public class LedgersIndexRebuildOp {
+ private static final Logger LOG = LoggerFactory.getLogger(LedgersIndexRebuildOp.class);
+
+ private final ServerConfiguration conf;
+ private final boolean verbose;
+ private static final String LedgersSubPath = "ledgers";
+
+ public LedgersIndexRebuildOp(ServerConfiguration conf, boolean verbose) {
+ this.conf = conf;
+ this.verbose = verbose;
+ }
+
+ public boolean initiate() {
+ LOG.info("Starting ledger index rebuilding");
+
+ String timestamp = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSSZ").format(new Date());
+ String basePath = BookieImpl.getCurrentDirectory(conf.getLedgerDirs()[0]).toString();
+ String tempLedgersSubPath = LedgersSubPath + ".TEMP-" + timestamp;
+ Path tempPath = FileSystems.getDefault().getPath(basePath, tempLedgersSubPath);
+ Path currentPath = FileSystems.getDefault().getPath(basePath, LedgersSubPath);
+
+ LOG.info("Starting scan phase (scans journal and entry log files)");
+
+ try {
+ Set<Long> ledgers = new HashSet<>();
+ scanJournals(ledgers);
+ scanEntryLogFiles(ledgers);
+
+ LOG.info("Scan complete, found {} ledgers. "
+ + "Starting to build a new ledgers index", ledgers.size());
+
+ KeyValueStorage newIndex = KeyValueStorageRocksDB.factory.newKeyValueStorage(
+ basePath, tempLedgersSubPath, DbConfigType.Small, conf);
+ LOG.info("Created ledgers index at temp location {}", tempPath);
+
+ for (Long ledgerId : ledgers) {
+ DbLedgerStorageDataFormats.LedgerData ledgerData =
+ DbLedgerStorageDataFormats.LedgerData.newBuilder()
+ .setExists(true)
+ .setFenced(true)
+ .setMasterKey(ByteString.EMPTY).build();
+
+ byte[] ledgerArray = new byte[16];
+ ArrayUtil.setLong(ledgerArray, 0, ledgerId);
+ newIndex.put(ledgerArray, ledgerData.toByteArray());
+ }
+
+ newIndex.sync();
+ newIndex.close();
+ } catch (Throwable t) {
+ LOG.error("Error during rebuild, the original index remains unchanged", t);
+ delete(tempPath);
+ return false;
+ }
+
+ // replace the existing index
+ try {
+ Path prevPath = FileSystems.getDefault().getPath(basePath, LedgersSubPath + ".PREV-" + timestamp);
+ Files.move(currentPath, prevPath);
+ Files.move(tempPath, currentPath);
Review comment:
what if this instruction fails? we don't have a valid index file anymore?
--
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: issues-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] Vanlightly commented on a change in pull request #2774: ISSUE-2773: Add db ledgers index rebuild op
Posted by GitBox <gi...@apache.org>.
Vanlightly commented on a change in pull request #2774:
URL: https://github.com/apache/bookkeeper/pull/2774#discussion_r695603700
##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/LedgersIndexRebuildOp.java
##########
@@ -0,0 +1,214 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+package org.apache.bookkeeper.bookie.storage.ldb;
+
+import com.google.common.collect.Lists;
+import com.google.protobuf.ByteString;
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.Unpooled;
+import java.io.File;
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.file.FileSystems;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.text.SimpleDateFormat;
+import java.util.Date;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import org.apache.bookkeeper.bookie.BookieImpl;
+import org.apache.bookkeeper.bookie.EntryLogger;
+import org.apache.bookkeeper.bookie.EntryLogger.EntryLogScanner;
+import org.apache.bookkeeper.bookie.Journal;
+import org.apache.bookkeeper.bookie.LedgerDirsManager;
+import org.apache.bookkeeper.bookie.storage.ldb.KeyValueStorageFactory.DbConfigType;
+import org.apache.bookkeeper.conf.ServerConfiguration;
+import org.apache.bookkeeper.util.BookKeeperConstants;
+import org.apache.bookkeeper.util.DiskChecker;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Scan all entries in the journal and entry log files then rebuilds the ledgers index.
+ * Notable stuff:
+ * - Fences every ledger as even if we check the metadata, we cannot guarantee that
+ * a fence request was served while the rebuild was taking place (even if the bookie
+ * is running in read-only mode).
+ * Losing the fenced status of a ledger is UNSAFE.
+ * - Sets the master key as an empty byte array. This is correct as empty master keys
+ * are overwritten and we cannot use the password from metadata, and cannot know 100%
+ * for sure how a digest for the password was generated.
+ */
+public class LedgersIndexRebuildOp {
+ private static final Logger LOG = LoggerFactory.getLogger(LedgersIndexRebuildOp.class);
+
+ private final ServerConfiguration conf;
+ private final boolean verbose;
+ private static final String LedgersSubPath = "ledgers";
+
+ public LedgersIndexRebuildOp(ServerConfiguration conf, boolean verbose) {
+ this.conf = conf;
+ this.verbose = verbose;
+ }
+
+ public boolean initiate() {
+ LOG.info("Starting ledger index rebuilding");
+
+ // Move locations index to a backup directory
+ String basePath = BookieImpl.getCurrentDirectory(conf.getLedgerDirs()[0]).toString();
+ Path currentPath = FileSystems.getDefault().getPath(basePath, LedgersSubPath);
+ String timestamp = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSSZ").format(new Date());
+ Path backupPath = FileSystems.getDefault().getPath(basePath, LedgersSubPath + ".BACKUP-" + timestamp);
+ Path abortPath = FileSystems.getDefault().getPath(basePath, LedgersSubPath + ".ABORT-" + timestamp);
+ try {
+ Files.move(currentPath, backupPath);
+ } catch (IOException e) {
+ LOG.error("Could not move index to a backup location", e);
+ return false;
+ }
+
+ LOG.info("Created ledgers index backup at {}", backupPath);
+ LOG.info("Starting scan phase (scans journal and entry log files)");
+
+ try {
+ Set<Long> ledgers = new HashSet<>();
+ scanJournals(ledgers);
+ scanEntryLogFiles(ledgers);
+
+ LOG.info("Scan complete, found {} ledgers. "
+ + "Starting to build a new ledgers index", ledgers.size());
+
+ KeyValueStorage newIndex = KeyValueStorageRocksDB.factory.newKeyValueStorage(
+ basePath, LedgersSubPath, DbConfigType.Small, conf);
+
+ for (Long ledgerId : ledgers) {
+ DbLedgerStorageDataFormats.LedgerData ledgerData =
+ DbLedgerStorageDataFormats.LedgerData.newBuilder()
+ .setExists(true)
+ .setFenced(true)
+ .setMasterKey(ByteString.EMPTY).build();
+
+ byte[] ledgerArray = new byte[16];
+ ArrayUtil.setLong(ledgerArray, 0, ledgerId);
+ newIndex.put(ledgerArray, ledgerData.toByteArray());
+ }
+
+ newIndex.sync();
+ newIndex.close();
+ } catch (Throwable t) {
+ LOG.error("Error during rebuild", t);
+ restoreBackup(currentPath, backupPath, abortPath);
Review comment:
Good point, I was copying from the LocationsIndexRebuildOp which does it that way. I have updated this rebuild to swap them at the end once a new index has been generated.
--
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: issues-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] Vanlightly commented on a change in pull request #2774: ISSUE-2773: Add db ledgers index rebuild op
Posted by GitBox <gi...@apache.org>.
Vanlightly commented on a change in pull request #2774:
URL: https://github.com/apache/bookkeeper/pull/2774#discussion_r714949539
##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/LedgersIndexRebuildOp.java
##########
@@ -0,0 +1,214 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+package org.apache.bookkeeper.bookie.storage.ldb;
+
+import com.google.common.collect.Lists;
+import com.google.protobuf.ByteString;
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.Unpooled;
+import java.io.File;
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.file.FileSystems;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.text.SimpleDateFormat;
+import java.util.Date;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import org.apache.bookkeeper.bookie.BookieImpl;
+import org.apache.bookkeeper.bookie.EntryLogger;
+import org.apache.bookkeeper.bookie.EntryLogger.EntryLogScanner;
+import org.apache.bookkeeper.bookie.Journal;
+import org.apache.bookkeeper.bookie.LedgerDirsManager;
+import org.apache.bookkeeper.bookie.storage.ldb.KeyValueStorageFactory.DbConfigType;
+import org.apache.bookkeeper.conf.ServerConfiguration;
+import org.apache.bookkeeper.util.BookKeeperConstants;
+import org.apache.bookkeeper.util.DiskChecker;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Scan all entries in the journal and entry log files then rebuilds the ledgers index.
+ * Notable stuff:
+ * - Fences every ledger as even if we check the metadata, we cannot guarantee that
+ * a fence request was served while the rebuild was taking place (even if the bookie
+ * is running in read-only mode).
+ * Losing the fenced status of a ledger is UNSAFE.
+ * - Sets the master key as an empty byte array. This is correct as empty master keys
+ * are overwritten and we cannot use the password from metadata, and cannot know 100%
+ * for sure how a digest for the password was generated.
+ */
+public class LedgersIndexRebuildOp {
+ private static final Logger LOG = LoggerFactory.getLogger(LedgersIndexRebuildOp.class);
+
+ private final ServerConfiguration conf;
+ private final boolean verbose;
+ private static final String LedgersSubPath = "ledgers";
+
+ public LedgersIndexRebuildOp(ServerConfiguration conf, boolean verbose) {
+ this.conf = conf;
+ this.verbose = verbose;
+ }
+
+ public boolean initiate() {
+ LOG.info("Starting ledger index rebuilding");
+
+ String timestamp = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSSZ").format(new Date());
+ String basePath = BookieImpl.getCurrentDirectory(conf.getLedgerDirs()[0]).toString();
+ String tempLedgersSubPath = LedgersSubPath + ".TEMP-" + timestamp;
+ Path tempPath = FileSystems.getDefault().getPath(basePath, tempLedgersSubPath);
+ Path currentPath = FileSystems.getDefault().getPath(basePath, LedgersSubPath);
+
+ LOG.info("Starting scan phase (scans journal and entry log files)");
+
+ try {
+ Set<Long> ledgers = new HashSet<>();
+ scanJournals(ledgers);
+ scanEntryLogFiles(ledgers);
+
+ LOG.info("Scan complete, found {} ledgers. "
+ + "Starting to build a new ledgers index", ledgers.size());
+
+ KeyValueStorage newIndex = KeyValueStorageRocksDB.factory.newKeyValueStorage(
Review comment:
Ok
##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/LedgersIndexCheckOp.java
##########
@@ -0,0 +1,102 @@
+/**
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+package org.apache.bookkeeper.bookie.storage.ldb;
+
+import java.io.IOException;
+import java.nio.file.FileSystems;
+import java.nio.file.Path;
+import java.util.Base64;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+import org.apache.bookkeeper.bookie.BookieImpl;
+import org.apache.bookkeeper.bookie.storage.ldb.KeyValueStorageFactory.DbConfigType;
+import org.apache.bookkeeper.conf.ServerConfiguration;
+import org.apache.commons.lang.time.DurationFormatUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Scan the ledgers index to make sure it is readable.
+ */
+public class LedgersIndexCheckOp {
+ private static final Logger LOG = LoggerFactory.getLogger(LedgersIndexCheckOp.class);
+
+ private final ServerConfiguration conf;
+ private final boolean verbose;
+ private static final String LedgersSubPath = "ledgers";
+
+ public LedgersIndexCheckOp(ServerConfiguration conf, boolean verbose) {
+ this.conf = conf;
+ this.verbose = verbose;
+ }
+
+ public boolean initiate() throws IOException {
+ String basePath = BookieImpl.getCurrentDirectory(conf.getLedgerDirs()[0]).toString();
+ Path currentPath = FileSystems.getDefault().getPath(basePath, LedgersSubPath);
+
+ LOG.info("Loading ledgers index from {}", currentPath);
+
+ long startTime = System.nanoTime();
+ LOG.info("Starting index scan");
+
+ try {
+ KeyValueStorage index = new KeyValueStorageRocksDB(basePath, LedgersSubPath,
+ DbConfigType.Small, conf, true);
+ // Read all ledgers from db
+ KeyValueStorage.CloseableIterator<Map.Entry<byte[], byte[]>> iterator = index.iterator();
+ int ctr = 0;
+ try {
+ while (iterator.hasNext()) {
+ ctr++;
+ Map.Entry<byte[], byte[]> entry = iterator.next();
+ long ledgerId = ArrayUtil.getLong(entry.getKey(), 0);
+ DbLedgerStorageDataFormats.LedgerData ledgerData =
+ DbLedgerStorageDataFormats.LedgerData.parseFrom(entry.getValue());
+ if (verbose) {
+ LOG.info("Ledger {}, exists: {}, isFenced: {}, masterKey: {}, explicitLAC: {}",
+ ledgerId,
+ (ledgerData.hasExists() ? ledgerData.getExists() : "-"),
+ (ledgerData.hasFenced() ? ledgerData.getFenced() : "-"),
+ (ledgerData.hasMasterKey()
+ ? Base64.getEncoder()
+ .encodeToString(ledgerData.getMasterKey().toByteArray())
+ : "-"),
+ (ledgerData.hasExplicitLac() ? ledgerData.getExplicitLac() : "-"));
+ } else if (ctr % 100 == 0) {
Review comment:
Ok
--
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: issues-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org