diff --git a/pixels-common/src/main/java/io/pixelsdb/pixels/common/metadata/MetadataService.java b/pixels-common/src/main/java/io/pixelsdb/pixels/common/metadata/MetadataService.java index 6486e7c0c..7847f51b6 100644 --- a/pixels-common/src/main/java/io/pixelsdb/pixels/common/metadata/MetadataService.java +++ b/pixels-common/src/main/java/io/pixelsdb/pixels/common/metadata/MetadataService.java @@ -1676,7 +1676,7 @@ public File getFileById(long fileId) throws MetadataException /** * Atomically promote a temporary GC file to REGULAR and retire the old files. - * @param newFileId the id of the new temporary GC file to promote + * @param newFileId the new temporary GC file to promote, or zero to retire without a replacement * @param oldFileIds the ids of old files to retire * @param cleanupAt the cleanup deadline to write on retired old files * @throws MetadataException if the request fails diff --git a/pixels-daemon/src/main/java/io/pixelsdb/pixels/daemon/metadata/dao/impl/RdbFileDao.java b/pixels-daemon/src/main/java/io/pixelsdb/pixels/daemon/metadata/dao/impl/RdbFileDao.java index 7b26383d9..73afb0fde 100644 --- a/pixels-daemon/src/main/java/io/pixelsdb/pixels/daemon/metadata/dao/impl/RdbFileDao.java +++ b/pixels-daemon/src/main/java/io/pixelsdb/pixels/daemon/metadata/dao/impl/RdbFileDao.java @@ -351,10 +351,13 @@ public boolean deleteByIds(List ids) @Override public boolean atomicSwapFiles(long newFileId, List oldFileIds, long cleanupAt) { + if (newFileId < 0 || (newFileId == 0 && (oldFileIds == null || oldFileIds.isEmpty()))) + return false; Connection conn = db.getConnection(); try { conn.setAutoCommit(false); + if (newFileId > 0) try (PreparedStatement pst = conn.prepareStatement( "UPDATE FILES SET FILE_TYPE=?, FILE_CLEANUP_AT=NULL WHERE FILE_ID=?")) { diff --git a/pixels-daemon/src/main/java/io/pixelsdb/pixels/daemon/retina/RetinaServerImpl.java b/pixels-daemon/src/main/java/io/pixelsdb/pixels/daemon/retina/RetinaServerImpl.java index 485c89f94..4b6aae509 100644 --- a/pixels-daemon/src/main/java/io/pixelsdb/pixels/daemon/retina/RetinaServerImpl.java +++ b/pixels-daemon/src/main/java/io/pixelsdb/pixels/daemon/retina/RetinaServerImpl.java @@ -261,7 +261,7 @@ private RecoveryResult recoverRetinaState(RecoveryContext context) throws Retina recoverableFileIds.add(fileId); } - context.storageGcWalRecoveryHandler.recover(recoverableFileIds); + context.storageGcWalRecoveryHandler.recover(recoverableFileIds, body.getCheckpointAppliedTs()); for (VisibilityEntry ve : validRgEntries) { diff --git a/pixels-daemon/src/test/java/io/pixelsdb/pixels/daemon/metadata/dao/TestRdbFileDao.java b/pixels-daemon/src/test/java/io/pixelsdb/pixels/daemon/metadata/dao/TestRdbFileDao.java index a70d57b0d..826bc2b5f 100644 --- a/pixels-daemon/src/test/java/io/pixelsdb/pixels/daemon/metadata/dao/TestRdbFileDao.java +++ b/pixels-daemon/src/test/java/io/pixelsdb/pixels/daemon/metadata/dao/TestRdbFileDao.java @@ -604,4 +604,26 @@ private static Connection swapConnection(Connection replacement) throws Exceptio f.set(MetaDBUtil.Instance(), replacement); return previous; } + + @Test + public void atomicSwapFiles_retiresFullyDeletedGroupWithoutReplacement() throws Exception + { + PreparedStatement statement = mock(PreparedStatement.class); + when(mockConn.prepareStatement(anyString())).thenReturn(statement); + assertTrue(dao.atomicSwapFiles(0, Arrays.asList(11L, 12L), 123456L)); + ArgumentCaptor sql = ArgumentCaptor.forClass(String.class); + verify(mockConn).prepareStatement(sql.capture()); + assertFalse(sql.getValue().contains("FILE_CLEANUP_AT=NULL")); + verify(statement).setLong(3, 11L); + verify(statement).setLong(3, 12L); + verify(statement).executeBatch(); + verify(mockConn).commit(); + } + + @Test + public void atomicSwapFiles_rejectsRetirementWithoutOldFiles() throws Exception + { + assertFalse(dao.atomicSwapFiles(0, Collections.emptyList(), 123456L)); + verify(mockConn, never()).commit(); + } } diff --git a/pixels-retina/src/main/java/io/pixelsdb/pixels/retina/RetinaResourceManager.java b/pixels-retina/src/main/java/io/pixelsdb/pixels/retina/RetinaResourceManager.java index f06ab3c08..b0d8c5b9e 100644 --- a/pixels-retina/src/main/java/io/pixelsdb/pixels/retina/RetinaResourceManager.java +++ b/pixels-retina/src/main/java/io/pixelsdb/pixels/retina/RetinaResourceManager.java @@ -412,9 +412,16 @@ public void processRetiredFiles() { // In-memory queue for files retired in this process. long now = System.currentTimeMillis(); + final Set pending; + try { pending = storageGcWal.collectPendingFileIds(); } + catch (RuntimeException e) + { + logger.warn("processRetiredFiles: cannot establish checkpoint protection; skipping cleanup", e); + return; + } retiredFiles.removeIf(rf -> { - if (now <= rf.retireTimestamp) + if (now <= rf.retireTimestamp || pending.contains(rf.fileId)) { return false; } @@ -459,6 +466,7 @@ public void processRetiredFiles() } for (File file : dueFiles) { + if (pending.contains(file.getId())) continue; RetiredPathInfo pathInfo = pathsById.get(file.getPathId()); if (pathInfo == null) { @@ -1041,7 +1049,7 @@ private void runGC() for (StorageGcWal.Task task : storageGcWal.listAllTasks()) { if (task.getState() != StorageGcWal.State.SWAPPED_NOT_CHECKPOINTED - || !checkpointFileIds.contains(task.getNewFileId())) + || !task.isCoveredByCheckpoint(checkpointFileIds, timestamp)) { continue; } diff --git a/pixels-retina/src/main/java/io/pixelsdb/pixels/retina/StorageGarbageCollector.java b/pixels-retina/src/main/java/io/pixelsdb/pixels/retina/StorageGarbageCollector.java index 2ac5a71fa..68cfd8cf7 100644 --- a/pixels-retina/src/main/java/io/pixelsdb/pixels/retina/StorageGarbageCollector.java +++ b/pixels-retina/src/main/java/io/pixelsdb/pixels/retina/StorageGarbageCollector.java @@ -64,6 +64,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Objects; import java.util.Set; import java.util.stream.Collectors; import org.apache.logging.log4j.LogManager; @@ -481,14 +482,16 @@ List groupAndMerge(List candidates) { int vNodeId = vnodeEntry.getKey(); List files = vnodeEntry.getValue(); - files.sort(Comparator.comparingDouble((FileCandidate c) -> c.invalidRatio).reversed()); + files.sort(Comparator.comparingDouble((FileCandidate c) -> c.invalidRatio).reversed() + .thenComparingLong(c -> c.fileId)); splitIntoGroups(groups, tableId, vNodeId, files); } } groups.sort(Comparator.comparingDouble( (FileGroup g) -> g.files.stream().mapToDouble(c -> c.invalidRatio).average().orElse(0.0)) - .reversed()); + .reversed().thenComparingLong(g -> g.files.stream().mapToLong(c -> c.fileId) + .min().orElse(Long.MAX_VALUE))); if (groups.size() > maxFileGroupsPerRun) { @@ -828,12 +831,10 @@ RewriteResult rewriteFileGroup(FileGroup group, long safeGcTs, } } - // Edge case: all rows in the group were deleted — skip catalog registration, - // delete the empty file, and return early. The old files will be cleaned up - // by the delayed-cleanup phase once it is implemented. + // Fully deleted groups need no replacement; the caller journals and retires the old files. if (globalNewRowOffset == 0) { - logger.info("StorageGC: all rows deleted for table={}, vNodeId={}, skipping empty file", + logger.info("StorageGC: all rows deleted for table={}, vNodeId={}, retiring without replacement", group.tableId, group.virtualNodeId); try { @@ -1414,6 +1415,68 @@ private void rollbackSinglePointIndex(RewriteResult result) // Per-group orchestration // ------------------------------------------------------------------------- + void retireEmptyFileGroup(RewriteResult result, long safeGcTs) throws Exception + { + if (result.newFileId > 0 || result.newFileRgCount != 0 || !result.pendingIndexEntries.isEmpty()) + throw new IllegalArgumentException("Retirement requires a fully deleted rewrite result"); + List oldFileIds = result.group.files.stream().map(fc -> fc.fileId).collect(Collectors.toList()); + String taskId = "empty_" + RetinaUtils.buildStorageGcJournalTaskId( + result.group.tableId, result.group.virtualNodeId, oldFileIds.get(0)); + long deadline = System.currentTimeMillis() + retireDelayMs; + result.walWriter = wal.createTask(taskId, result.group.tableId, result.group.virtualNodeId, + oldFileIds, 0, "", 0, 0); + try + { + result.walWriter.appendRetirementOnly(safeGcTs); + result.walWriter.flush(); + try + { + metadataService.atomicSwapFiles(0, oldFileIds, deadline); + } + catch (Exception e) + { + for (long id : oldFileIds) + { + File file = metadataService.getFileById(id); + if (file == null || file.getType() != File.Type.RETIRED + || !Objects.equals(file.getCleanupAt(), deadline)) throw e; + } + } + result.walWriter.markSwapped(); + result.walWriter = null; + for (FileCandidate file : result.group.files) + resourceManager.scheduleRetiredFile(new RetinaResourceManager.RetiredFile( + file.fileId, file.rgCount, file.filePath, deadline, Collections.emptyList())); + } + catch (Exception e) + { + try + { + for (long id : oldFileIds) + { + File file = metadataService.getFileById(id); + if (file == null) throw new MetadataException("Missing retirement rollback file " + id); + if (file.getType() == File.Type.RETIRED) + { + file.setType(File.Type.REGULAR); + file.setCleanupAt(null); + if (!metadataService.updateFile(file)) + throw new MetadataException("Cannot roll back retirement file " + id); + } + } + if (result.walWriter != null) result.walWriter.markAborted(); + } + catch (Exception rollbackError) { e.addSuppressed(rollbackError); } + finally + { + if (result.walWriter != null) result.walWriter.close(); + result.walWriter = null; + } + throw e; + } + } + + /** * Processes a single file group through the complete GC pipeline: * rewrite → dual-write → visibility sync → index sync → commit. @@ -1429,6 +1492,9 @@ void processFileGroup(FileGroup group, long safeGcTs, if (result.newFileId <= 0) { + retireEmptyFileGroup(result, safeGcTs); + logger.info("StorageGC completed for FileGroup tableId={}, vNodeId={}, retirementOnly=true", + group.tableId, group.virtualNodeId); return; } diff --git a/pixels-retina/src/main/java/io/pixelsdb/pixels/retina/StorageGcWal.java b/pixels-retina/src/main/java/io/pixelsdb/pixels/retina/StorageGcWal.java index 4d0423497..3971feb48 100644 --- a/pixels-retina/src/main/java/io/pixelsdb/pixels/retina/StorageGcWal.java +++ b/pixels-retina/src/main/java/io/pixelsdb/pixels/retina/StorageGcWal.java @@ -91,6 +91,7 @@ public final class StorageGcWal static final byte RT_CREATE = 1; static final byte RT_ROLLBACK_ENTRY = 2; static final byte RT_STATE_TRANSITION = 3; + static final byte RT_RETIREMENT_ONLY = 4; // ─── State ────────────────────────────────────────────────────────────── @@ -149,11 +150,12 @@ public static final class Task private final int newRowCount; private final List rollbackEntries; private final State state; + private final long retirementSafeGcTs; Task(String taskId, long tableId, int virtualNodeId, List oldFileIds, long newFileId, String newFilePath, long newRowIdStart, int newRowCount, - List rollbackEntries, State state) + List rollbackEntries, State state, long retirementSafeGcTs) { this.taskId = taskId; this.tableId = tableId; @@ -165,6 +167,7 @@ public static final class Task this.newRowCount = newRowCount; this.rollbackEntries = Collections.unmodifiableList(new ArrayList<>(rollbackEntries)); this.state = state; + this.retirementSafeGcTs = retirementSafeGcTs; } public String getTaskId() { return taskId; } @@ -177,6 +180,14 @@ public static final class Task public int getNewRowCount() { return newRowCount; } public List getRollbackEntries() { return rollbackEntries; } public State getState() { return state; } + public boolean isRetirementOnly() { return newFileId == 0 && newRowCount == 0; } + public long getRetirementSafeGcTs() { return retirementSafeGcTs; } + public boolean isCoveredByCheckpoint(Set visibleFileIds, long checkpointAppliedTs) + { + return isRetirementOnly() + ? retirementSafeGcTs >= 0 && checkpointAppliedTs >= retirementSafeGcTs + : visibleFileIds.contains(newFileId); + } } // ─── Writer (per-task write handle) ───────────────────────────────────── @@ -230,6 +241,15 @@ public void flush() throws IOException out.flush(); } + /** Records the visibility-fold timestamp before retiring a fully deleted group. */ + public void appendRetirementOnly(long safeGcTs) throws IOException + { + if (safeGcTs < 0) throw new IllegalArgumentException("Negative retirement timestamp"); + out.writeByte(RT_RETIREMENT_ONLY); + out.writeInt(Long.BYTES); + out.writeLong(safeGcTs); + } + /** Writes {@code SWAPPED_NOT_CHECKPOINTED} record and closes the stream. */ public void markSwapped() throws IOException { @@ -397,7 +417,7 @@ public Set collectPendingFileIds() { if (task.getState().isPending()) { - fileIds.add(task.getNewFileId()); + if (task.getNewFileId() > 0) fileIds.add(task.getNewFileId()); fileIds.addAll(task.getOldFileIds()); } } @@ -540,6 +560,7 @@ private Task replayWal(String path) throws IOException // ── Subsequent records (rollback entries + state transitions) ── List rollbackEntries = new ArrayList<>(); State state = State.INDEX_SWITCHING; + long retirementSafeGcTs = -1; while (true) { @@ -575,6 +596,14 @@ private Task replayWal(String path) throws IOException rollbackEntries.add(new RollbackEntry( IndexProto.IndexKey.parseFrom(keyBytes), oldRowId, newRowId)); } + else if ((byte) rt == RT_RETIREMENT_ONLY) + { + if (newFileId != 0 || newRowCount != 0 || rLen != Long.BYTES) + throw new IllegalStateException("Invalid retirement-only WAL record"); + retirementSafeGcTs = new DataInputStream(new ByteArrayInputStream(rPayload)).readLong(); + if (retirementSafeGcTs < 0) + throw new IllegalStateException("Negative retirement timestamp in WAL"); + } else if ((byte) rt == RT_STATE_TRANSITION) { state = State.fromCode(rPayload[0]); @@ -587,7 +616,7 @@ else if ((byte) rt == RT_STATE_TRANSITION) return new Task(taskId, tableId, virtualNodeId, oldFileIds, newFileId, newFilePath, newRowIdStart, newRowCount, - rollbackEntries, state); + rollbackEntries, state, retirementSafeGcTs); } } @@ -614,6 +643,11 @@ public RecoveryHandler(StorageGcWal wal, } public void recover(Set baselineVisibleFileIds) throws RetinaException + { + recover(baselineVisibleFileIds, -1L); + } + + public void recover(Set baselineVisibleFileIds, long checkpointAppliedTs) throws RetinaException { Set baseline = baselineVisibleFileIds == null ? Collections.emptySet() : baselineVisibleFileIds; @@ -626,6 +660,11 @@ public void recover(Set baselineVisibleFileIds) throws RetinaException // back, restoring old files only when the catalog confirms the swap committed. for (Task task : wal.listAllTasks()) { + if (task.isRetirementOnly()) + { + recoverRetirement(task, checkpointAppliedTs); + continue; + } switch (task.getState()) { case CHECKPOINTED: @@ -692,6 +731,26 @@ public void recover(Set baselineVisibleFileIds) throws RetinaException } } + private void recoverRetirement(Task task, long checkpointAppliedTs) throws RetinaException + { + boolean covered = task.isCoveredByCheckpoint(Collections.emptySet(), checkpointAppliedTs); + if (task.getState() == State.ABORTED) return; + if (task.getState() == State.CHECKPOINTED) + { + if (!covered) throw new RetinaException("Checkpoint predates retirement task " + task.getTaskId()); + return; + } + if (task.getState() == State.SWAPPED_NOT_CHECKPOINTED && covered) + { + try { wal.markCheckpointed(task.getTaskId()); } + catch (IOException e) { throw new RetinaException("Cannot checkpoint retirement " + task.getTaskId(), e); } + return; + } + if (task.getState() == State.INDEX_SWITCHING && covered) + throw new RetinaException("Unfinished retirement appears in checkpoint: " + task.getTaskId()); + rollbackTask(task, isSwapCommitted(task)); + } + private void rollbackTask(Task task, boolean restoreOldFiles) throws RetinaException { restorePrimaryIndex(task); @@ -784,6 +843,7 @@ private void restoreOldFileCatalog(Task task) throws RetinaException private void cleanupNewFile(Task task) throws RetinaException { + if (task.isRetirementOnly()) return; if (task.getNewRowCount() > 0) { try @@ -847,7 +907,7 @@ private void cleanupNewFile(Task task) throws RetinaException private boolean isSwapCommitted(Task task) throws RetinaException { - File newFile = loadOptionalFile(task.getNewFileId()); + File newFile = task.isRetirementOnly() ? null : loadOptionalFile(task.getNewFileId()); if (newFile != null && newFile.getType() == File.Type.REGULAR) { return true; diff --git a/pixels-retina/src/test/java/io/pixelsdb/pixels/retina/TestEmptyStorageGc.java b/pixels-retina/src/test/java/io/pixelsdb/pixels/retina/TestEmptyStorageGc.java new file mode 100644 index 000000000..c458ef2ae --- /dev/null +++ b/pixels-retina/src/test/java/io/pixelsdb/pixels/retina/TestEmptyStorageGc.java @@ -0,0 +1,95 @@ +package io.pixelsdb.pixels.retina; + +import io.pixelsdb.pixels.common.exception.MetadataException; +import io.pixelsdb.pixels.common.index.service.IndexService; +import io.pixelsdb.pixels.common.metadata.MetadataService; +import io.pixelsdb.pixels.common.metadata.domain.File; +import io.pixelsdb.pixels.common.utils.ConfigFactory; +import io.pixelsdb.pixels.core.encoding.EncodingLevel; +import org.junit.Test; +import java.nio.file.Files; +import java.util.Arrays; +import java.util.Collections; +import static org.junit.Assert.*; +import static org.mockito.ArgumentMatchers.*; +import static org.mockito.Mockito.*; + +public class TestEmptyStorageGc +{ + private StorageGarbageCollector.RewriteResult emptyResult() + { + File file = new File(); + file.setId(101); + file.setType(File.Type.REGULAR); + StorageGarbageCollector.FileCandidate candidate = new StorageGarbageCollector.FileCandidate( + file, "file:///tmp/obsolete.pxl", 101, 1, 11, 2, 1.0, 100); + return new StorageGarbageCollector.RewriteResult( + new StorageGarbageCollector.FileGroup(11, 2, Collections.singletonList(candidate)), + "", -1, 0, new int[0], new int[]{0}, Collections.emptyMap(), + Collections.emptyList(), Collections.emptyList()); + } + + @Test + public void fullyDeletedGroupRetiresWithoutReplacementOrIndexWrites() throws Exception + { + ConfigFactory.Instance().addProperty("retina.storage.gc.journal.dir", + Files.createTempDirectory("empty-gc-").toUri().toString()); + StorageGcWal wal = new StorageGcWal(); + RetinaResourceManager resource = mock(RetinaResourceManager.class); + MetadataService metadata = mock(MetadataService.class); + IndexService index = mock(IndexService.class); + StorageGarbageCollector gc = new StorageGarbageCollector(resource, metadata, index, + .5, 1024, 20, 10, 10240, EncodingLevel.from(2), 600000, wal); + gc.retireEmptyFileGroup(emptyResult(), 500); + verify(metadata).atomicSwapFiles(eq(0L), eq(Collections.singletonList(101L)), anyLong()); + verify(resource).scheduleRetiredFile(any(RetinaResourceManager.RetiredFile.class)); + verifyNoInteractions(index); + StorageGcWal.Task task = wal.listAllTasks().get(0); + assertTrue(task.isRetirementOnly()); + assertEquals(500, task.getRetirementSafeGcTs()); + assertEquals(StorageGcWal.State.SWAPPED_NOT_CHECKPOINTED, task.getState()); + assertEquals(Collections.singleton(101L), wal.collectPendingFileIds()); + } + + @Test + public void failedCatalogRetirementAbortsJournalAndQueuesNoDeletion() throws Exception + { + ConfigFactory.Instance().addProperty("retina.storage.gc.journal.dir", + Files.createTempDirectory("empty-gc-failed-").toUri().toString()); + StorageGcWal wal = new StorageGcWal(); + RetinaResourceManager resource = mock(RetinaResourceManager.class); + MetadataService metadata = mock(MetadataService.class); + IndexService index = mock(IndexService.class); + File old = new File(); + old.setId(101); old.setType(File.Type.REGULAR); + when(metadata.getFileById(101)).thenReturn(old); + doThrow(new MetadataException("Injected retirement failure")) + .when(metadata).atomicSwapFiles(eq(0L), anyList(), anyLong()); + StorageGarbageCollector gc = new StorageGarbageCollector(resource, metadata, index, + .5, 1024, 20, 10, 10240, EncodingLevel.from(2), 600000, wal); + try + { + gc.retireEmptyFileGroup(emptyResult(), 500); + fail("Expected injected failure"); + } + catch (MetadataException expected) { assertTrue(expected.getMessage().contains("Injected")); } + verifyNoInteractions(resource, index); + assertEquals(StorageGcWal.State.ABORTED, wal.listAllTasks().get(0).getState()); + assertTrue(wal.collectPendingFileIds().isEmpty()); + assertEquals(File.Type.REGULAR, old.getType()); + } + + @Test + public void equallyInvalidGroupsChooseOldestFilesBeforeNewlyCreatedGroups() throws Exception + { + StorageGarbageCollector gc = new StorageGarbageCollector(null, null, null, + .5, 1024, 20, 1, 10240, EncodingLevel.from(2), 600000, null); + File newer = new File(); newer.setId(200); + File older = new File(); older.setId(100); + java.util.List groups = gc.groupAndMerge(Arrays.asList( + new StorageGarbageCollector.FileCandidate(newer, "", 200, 1, 11, 0, 1.0, 100), + new StorageGarbageCollector.FileCandidate(older, "", 100, 1, 11, 1, 1.0, 100))); + assertEquals(1, groups.size()); + assertEquals(100, groups.get(0).files.get(0).fileId); + } +} diff --git a/pixels-retina/src/test/java/io/pixelsdb/pixels/retina/TestStorageGcWal.java b/pixels-retina/src/test/java/io/pixelsdb/pixels/retina/TestStorageGcWal.java index 4e2a895a9..f463b200e 100644 --- a/pixels-retina/src/test/java/io/pixelsdb/pixels/retina/TestStorageGcWal.java +++ b/pixels-retina/src/test/java/io/pixelsdb/pixels/retina/TestStorageGcWal.java @@ -47,13 +47,16 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertNull; import static org.junit.Assert.fail; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; /** @@ -621,4 +624,73 @@ private static IndexProto.IndexKey key(String value) .setTimestamp(33L) .build(); } + + @Test + public void retirementUsesCheckpointTimestampWithoutReplacement() throws Exception + { + StorageGcWal wal = newWal(); + try (StorageGcWal.Writer writer = wal.createTask("empty", 11, 2, + Collections.singletonList(101L), 0, "", 0, 0)) + { + writer.appendRetirementOnly(500); + writer.flush(); + writer.markSwapped(); + } + StorageGcWal.Task task = wal.getTask("empty").get(); + assertTrue(task.isRetirementOnly()); + assertEquals(500, task.getRetirementSafeGcTs()); + assertFalse(task.isCoveredByCheckpoint(Collections.emptySet(), 499)); + assertTrue(task.isCoveredByCheckpoint(Collections.emptySet(), 500)); + assertFalse(wal.collectPendingFileIds().contains(0L)); + assertTrue(wal.collectPendingFileIds().contains(101L)); + MetadataService metadata = mock(MetadataService.class); + IndexService index = mock(IndexService.class); + new StorageGcWal.RecoveryHandler(wal, metadata, index).recover(Collections.emptySet(), 500); + assertEquals(StorageGcWal.State.CHECKPOINTED, wal.getTask("empty").get().getState()); + verifyNoInteractions(metadata, index); + } + + @Test + public void retirementBeforeCheckpointRollsBackCatalogOnly() throws Exception + { + StorageGcWal wal = newWal(); + try (StorageGcWal.Writer writer = wal.createTask("empty-rollback", 11, 2, + Collections.singletonList(101L), 0, "", 0, 0)) + { + writer.appendRetirementOnly(500); + writer.markSwapped(); + } + MetadataService metadata = mock(MetadataService.class); + IndexService index = mock(IndexService.class); + File old = catalogFile(101L, File.Type.RETIRED, 123456L); + when(metadata.getFileById(101)).thenReturn(old); + when(metadata.updateFile(any(File.class))).thenReturn(true); + new StorageGcWal.RecoveryHandler(wal, metadata, index).recover(Collections.emptySet(), 499); + assertEquals(File.Type.REGULAR, old.getType()); + assertNull(old.getCleanupAt()); + assertEquals(StorageGcWal.State.ABORTED, wal.getTask("empty-rollback").get().getState()); + verify(index, never()).deleteMainIndexRange(anyLong(), anyLong(), anyLong(), anyInt()); + verify(metadata, never()).getFileById(0); + } + + @Test + public void retirementRejectsOlderCheckpointAfterConfirmation() throws Exception + { + StorageGcWal wal = newWal(); + try (StorageGcWal.Writer writer = wal.createTask("empty-confirmed", 11, 2, + Collections.singletonList(101L), 0, "", 0, 0)) + { + writer.appendRetirementOnly(500); + writer.markSwapped(); + } + wal.markCheckpointed("empty-confirmed"); + StorageGcWal.RecoveryHandler handler = new StorageGcWal.RecoveryHandler( + wal, mock(MetadataService.class), mock(IndexService.class)); + try + { + handler.recover(Collections.emptySet(), 499); + fail("Older checkpoint must not accept a confirmed retirement"); + } + catch (RetinaException expected) { assertTrue(expected.getMessage().contains("predates")); } + } } diff --git a/proto/metadata.proto b/proto/metadata.proto index 9cf726994..73019164e 100644 --- a/proto/metadata.proto +++ b/proto/metadata.proto @@ -744,6 +744,7 @@ message GetFileByIdResponse { message AtomicSwapFilesRequest { RequestHeader header = 1; + // Zero retires old files without a replacement for a fully deleted group. uint64 newFileId = 2; repeated uint64 oldFileIds = 3; optional uint64 cleanupAt = 4;