athanatos commented on a change in pull request #1819: ISSUE-1770: Add local
checker for Sorted/InterleavedLedgerStorage
URL: https://github.com/apache/bookkeeper/pull/1819#discussion_r239953827
##########
File path:
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java
##########
@@ -693,26 +701,101 @@ static long logIdForOffset(long offset) {
- public ByteBuf internalReadEntry(long ledgerId, long entryId, long
location)
- throws IOException, Bookie.NoEntryException {
- long entryLogId = logIdForOffset(location);
- long pos = location & 0xffffffffL;
+ /**
+ * Exception type for representing lookup errors. Useful for
disambiguating different error
+ * conditions for reporting purposes.
+ */
+ static class EntryLookupException extends Exception {
+ EntryLookupException(String message) {
+ super(message);
+ }
+
+ /**
+ * Represents case where log file is missing.
+ */
+ static class MissingLogFileException extends EntryLookupException {
+ MissingLogFileException(long ledgerId, long entryId, long
entryLogId, long pos) {
+ super(String.format("Missing entryLog %d for ledgerId %d,
entry %d at offset %d",
+ entryLogId,
+ ledgerId,
+ entryId,
+ pos));
+ }
+ }
+
+ /**
+ * Represents case where entry log is present, but does not contain
the specified entry.
+ */
+ static class MissingEntryException extends EntryLookupException {
+ MissingEntryException(long ledgerId, long entryId, long
entryLogId, long pos) {
+ super(String.format("pos %d (entry %d for ledgerId %d) past
end of entryLog %d",
+ pos,
+ entryId,
+ ledgerId,
+ entryLogId));
+ }
+ }
+
+ /**
+ * Represents case where log is present, but encoded entry length
header is invalid.
+ */
+ static class InvalidEntryLengthException extends EntryLookupException {
+ InvalidEntryLengthException(long ledgerId, long entryId, long
entryLogId, long pos) {
+ super(String.format("Invalid entry length at pos %d (entry %d
for ledgerId %d) for entryLog %d",
+ pos,
+ entryId,
+ ledgerId,
+ entryLogId));
+ }
+ }
+
+ /**
+ * Represents case where the entry at pos is wrong.
+ */
+ static class WrongEntryException extends EntryLookupException {
+ WrongEntryException(long foundEntryId, long foundLedgerId, long
ledgerId,
+ long entryId, long entryLogId, long pos) {
+ super(String.format(
+ "Found entry %d, ledger %d at pos %d entryLog %d,
should have found entry %d for ledgerId %d",
+ foundEntryId,
+ foundLedgerId,
+ pos,
+ entryLogId,
+ entryId,
+ ledgerId));
+ }
+ }
+ }
+
+ private static class EntryLogEntry {
+ final int entrySize;
+ final BufferedReadChannel fc;
+
+ EntryLogEntry(int entrySize, BufferedReadChannel fc) {
+ this.entrySize = entrySize;
+ this.fc = fc;
+ }
+ }
+
+ private EntryLogEntry getFCForEntryInternal(
+ long ledgerId, long entryId, long entryLogId, long pos)
+ throws EntryLookupException, IOException {
ByteBuf sizeBuff = sizeBuffer.get();
sizeBuff.clear();
pos -= 4; // we want to get the ledgerId and length to check
BufferedReadChannel fc;
try {
fc = getChannelForLogId(entryLogId);
} catch (FileNotFoundException e) {
- FileNotFoundException newe = new
FileNotFoundException(e.getMessage() + " for " + ledgerId
- + " with location " + location);
- newe.setStackTrace(e.getStackTrace());
- throw newe;
+ throw new EntryLookupException.MissingLogFileException(ledgerId,
entryId, entryLogId, pos);
}
- if (readFromLogChannel(entryLogId, fc, sizeBuff, pos) !=
sizeBuff.capacity()) {
- throw new Bookie.NoEntryException("Short read from entrylog " +
entryLogId,
- ledgerId, entryId);
+ try {
+ if (readFromLogChannel(entryLogId, fc, sizeBuff, pos) !=
sizeBuff.capacity()) {
+ throw new EntryLookupException.MissingEntryException(ledgerId,
entryId, entryLogId, pos);
+ }
+ } catch (BufferedChannelBase.ChannelClosed |
AsynchronousCloseException e) {
+ throw new EntryLookupException.MissingLogFileException(ledgerId,
entryId, entryLogId, pos);
Review comment:
AFAICT, it only happens if the deletion of the EntryLog races with that
read. It's not really common prior to this patch, but this implementation can
easily be checking an EntryLog as it gets deleted.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
