Txn compaction recovery can leave entry logs unreclaimed after restart

[void-ptr974]

Problem

Txn compaction recovery can create the recovered entry log with a decimal file name, while DefaultEntryLogger later resolves the same log id with the normal hex file name.

Entry log file names are hex-based. For log id 500, the expected file name is 1f4.log.

Code path

Normal txn compaction keeps the hex file name when it builds finalLogFile:

bookkeeper/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/DefaultEntryLogger.java

Lines 1316 to 1326 in cbb3367

	public CompactionEntryLog newCompactionLog(long logToCompact) throws IOException {
	createNewCompactionLog();
	File compactingLogFile = getCurCompactionLogFile();
	long compactionLogId = fileName2LogId(compactingLogFile.getName());
	File compactedLogFile = compactedLogFileFromCompacting(compactingLogFile, logToCompact);
	File finalLogFile = new File(compactingLogFile.getParentFile(),
	compactingLogFile.getName().substring(0,
	compactingLogFile.getName().indexOf(".log") + 4));
	return new EntryLoggerCompactionEntryLog(
	compactionLogId, logToCompact, compactingLogFile, compactedLogFile, finalLogFile);

makeAvailable() creates the final entry log at finalLogFile:

bookkeeper/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/DefaultEntryLogger.java

Lines 1370 to 1386 in cbb3367

	@Override
	public void markCompacted() throws IOException {
	if (compactingLogFile.exists()) {
	if (!compactedLogFile.exists()) {
	HardLink.createHardLink(compactingLogFile, compactedLogFile);
	}
	} else {
	throw new IOException("Compaction log doesn't exist any more after flush: " + compactingLogFile);
	}
	removeCurCompactionLog();
	}
	@Override
	public void makeAvailable() throws IOException {
	if (!finalLogFile.exists()) {
	HardLink.createHardLink(compactedLogFile, finalLogFile);
	}

Restart recovery rebuilds finalLogFile from a *.compacted file:

bookkeeper/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/DefaultEntryLogger.java

Lines 1424 to 1478 in cbb3367

	public Collection<CompactionEntryLog> incompleteCompactionLogs() {
	List<File> ledgerDirs = ledgerDirsManager.getAllLedgerDirs();
	List<CompactionEntryLog> compactionLogs = new ArrayList<>();
	for (File dir : ledgerDirs) {
	File[] compactingPhaseFiles = dir.listFiles(
	file -> file.getName().endsWith(TransactionalEntryLogCompactor.COMPACTING_SUFFIX));
	if (compactingPhaseFiles != null) {
	for (File file : compactingPhaseFiles) {
	if (file.delete()) {
	log.info().attr("file", file).log("Deleted failed compaction file");
	}
	}
	}
	File[] compactedPhaseFiles = dir.listFiles(
	file -> file.getName().endsWith(TransactionalEntryLogCompactor.COMPACTED_SUFFIX));
	if (compactedPhaseFiles != null) {
	for (File compactedFile : compactedPhaseFiles) {
	log.info()
	.attr("compactedFile", compactedFile)
	.log("Found compacted log file has partially flushed index, recovering index.");
	File compactingLogFile = new File(compactedFile.getParentFile(), "doesntexist");
	long compactionLogId = -1L;
	long compactedLogId = -1L;
	String[] parts = compactedFile.getName().split(Pattern.quote("."));
	boolean valid = true;
	if (parts.length != 4) {
	valid = false;
	} else {
	try {
	compactionLogId = Long.parseLong(parts[0], 16);
	compactedLogId = Long.parseLong(parts[2], 16);
	} catch (NumberFormatException nfe) {
	valid = false;
	}
	}
	if (!valid) {
	log.info()
	.attr("compactedFile", compactedFile)
	.log("Invalid compacted file found (), deleting");
	if (!compactedFile.delete()) {
	log.warn()
	.attr("compactedFile", compactedFile)
	.log("Couldn't delete invalid compacted file");
	}
	continue;
	}
	File finalLogFile = new File(compactedFile.getParentFile(), compactionLogId + ".log");
	compactionLogs.add(
	new EntryLoggerCompactionEntryLog(compactionLogId, compactedLogId,
	compactingLogFile, compactedFile, finalLogFile));
	}

Later scan resolves a numeric log id back to the hex file name:

bookkeeper/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/DefaultEntryLogger.java

Lines 977 to 1005 in cbb3367

	private File findFile(long logId) throws FileNotFoundException {
	for (File d : ledgerDirsManager.getAllLedgerDirs()) {
	File f = new File(d, Long.toHexString(logId) + ".log");
	if (f.exists()) {
	return f;
	}
	}
	throw new FileNotFoundException("No file for log " + Long.toHexString(logId));
	}
	/**
	* Scan entry log.
	*
	* @param entryLogId Entry Log Id
	* @param scanner Entry Log Scanner
	* @throws IOException
	*/
	@Override
	public void scanEntryLog(long entryLogId, EntryLogScanner scanner) throws IOException {
	// Buffer where to read the entrySize (4 bytes) and the ledgerId (8 bytes)
	ByteBuf headerBuffer = Unpooled.buffer(4 + 8);
	BufferedReadChannel bc;
	// Get the BufferedChannel for the current entry log file
	try {
	bc = getChannelForLogId(entryLogId);
	} catch (IOException e) {
	log.warn().attr("entryLogId", entryLogId).log("Failed to get channel to scan entry log");
	throw e;
	}

Example

Assume restart recovery sees this pending compaction file:

1f4.log.78.compacted

• 1f4 is the destination log id, hex for decimal 500.
• recovery parses 1f4 as 500.
• recovery builds the final file name from the numeric id and targets 500.log.
• makeAvailable() creates 500.log.
• later scan for log id 500 looks for 1f4.log.

The created file and the lookup file do not match.

Impact

Txn compaction recovery may not scan and finalize the recovered log. The .compacted marker and old source entry log can remain on disk, so disk space for that compaction is not reclaimed.

Ledger entry contents are not rewritten or changed.

Fix

Rebuild the recovered final .log file name with the same hex format used by normal entry logs.

The fix also covers the upgrade case where an older recovery attempt already left a stale decimal-named log file while the .compacted marker is still present.