IGNITE-8531 Fixed NPE if checkpoint has no pages to write, but has partitions to destroy. - Fixes #4026.
Signed-off-by: Alexey Goncharuk <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/ignite/repo Commit: http://git-wip-us.apache.org/repos/asf/ignite/commit/5c8d9ffa Tree: http://git-wip-us.apache.org/repos/asf/ignite/tree/5c8d9ffa Diff: http://git-wip-us.apache.org/repos/asf/ignite/diff/5c8d9ffa Branch: refs/heads/ignite-5789-1 Commit: 5c8d9ffa20fbe4497c5cd96a11f315df2baa9ba4 Parents: c2285c7 Author: Pavel Kovalenko <[email protected]> Authored: Fri May 18 19:28:45 2018 +0300 Committer: Alexey Goncharuk <[email protected]> Committed: Fri May 18 19:28:45 2018 +0300 ---------------------------------------------------------------------- .../GridCacheDatabaseSharedManager.java | 24 ++++++++++------- .../IgnitePdsPartitionFilesDestroyTest.java | 28 ++++++++++++++++++++ 2 files changed, 43 insertions(+), 9 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/ignite/blob/5c8d9ffa/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/GridCacheDatabaseSharedManager.java ---------------------------------------------------------------------- diff --git a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/GridCacheDatabaseSharedManager.java b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/GridCacheDatabaseSharedManager.java index 7151d75..2eb6e6f 100755 --- a/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/GridCacheDatabaseSharedManager.java +++ b/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/persistence/GridCacheDatabaseSharedManager.java @@ -3110,6 +3110,8 @@ public class GridCacheDatabaseSharedManager extends IgniteCacheDatabaseSharedMan boolean success = false; + int destroyedPartitionsCnt; + try { if (chp.hasDelta()) { // Identity stores set. @@ -3196,7 +3198,7 @@ public class GridCacheDatabaseSharedManager extends IgniteCacheDatabaseSharedMan snapshotMgr.afterCheckpointPageWritten(); try { - destroyEvictedPartitions(); + destroyedPartitionsCnt = destroyEvictedPartitions(); } catch (IgniteCheckedException e) { chp.progress.cpFinishFut.onDone(e); @@ -3216,15 +3218,15 @@ public class GridCacheDatabaseSharedManager extends IgniteCacheDatabaseSharedMan tracker.onEnd(); - if (chp.hasDelta()) { + if (chp.hasDelta() || destroyedPartitionsCnt > 0) { if (printCheckpointStats) { if (log.isInfoEnabled()) log.info(String.format("Checkpoint finished [cpId=%s, pages=%d, markPos=%s, " + "walSegmentsCleared=%d, markDuration=%dms, pagesWrite=%dms, fsync=%dms, " + "total=%dms]", - chp.cpEntry.checkpointId(), + chp.cpEntry != null ? chp.cpEntry.checkpointId() : "", chp.pagesSize, - chp.cpEntry.checkpointMark(), + chp.cpEntry != null ? chp.cpEntry.checkpointMark() : "", chp.walFilesDeleted, tracker.markDuration(), tracker.pagesWriteDuration(), @@ -3264,12 +3266,14 @@ public class GridCacheDatabaseSharedManager extends IgniteCacheDatabaseSharedMan * Processes all evicted partitions scheduled for destroy. * * @throws IgniteCheckedException If failed. + * + * @return The number of destroyed partition files. */ - private void destroyEvictedPartitions() throws IgniteCheckedException { + private int destroyEvictedPartitions() throws IgniteCheckedException { PartitionDestroyQueue destroyQueue = curCpProgress.destroyQueue; if (destroyQueue.pendingReqs.isEmpty()) - return; + return 0; List<PartitionDestroyRequest> reqs = null; @@ -3327,6 +3331,8 @@ public class GridCacheDatabaseSharedManager extends IgniteCacheDatabaseSharedMan req.waitCompleted(); destroyQueue.pendingReqs.clear(); + + return reqs != null ? reqs.size() : 0; } /** @@ -3495,7 +3501,7 @@ public class GridCacheDatabaseSharedManager extends IgniteCacheDatabaseSharedMan hasPages = hasPageForWrite(cpPagesTuple.get1()); - if (hasPages || curr.nextSnapshot) { + if (hasPages || curr.nextSnapshot || !curr.destroyQueue.pendingReqs.isEmpty()) { // No page updates for this checkpoint are allowed from now on. cpPtr = cctx.wal().log(cpRec); @@ -3874,7 +3880,7 @@ public class GridCacheDatabaseSharedManager extends IgniteCacheDatabaseSharedMan */ private static class Checkpoint { /** Checkpoint entry. */ - private final CheckpointEntry cpEntry; + @Nullable private final CheckpointEntry cpEntry; /** Checkpoint pages. */ private final GridMultiCollectionWrapper<FullPageId> cpPages; @@ -3894,7 +3900,7 @@ public class GridCacheDatabaseSharedManager extends IgniteCacheDatabaseSharedMan * @param progress Checkpoint progress status. */ private Checkpoint( - CheckpointEntry cpEntry, + @Nullable CheckpointEntry cpEntry, @NotNull GridMultiCollectionWrapper<FullPageId> cpPages, CheckpointProgress progress ) { http://git-wip-us.apache.org/repos/asf/ignite/blob/5c8d9ffa/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/IgnitePdsPartitionFilesDestroyTest.java ---------------------------------------------------------------------- diff --git a/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/IgnitePdsPartitionFilesDestroyTest.java b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/IgnitePdsPartitionFilesDestroyTest.java index b5afddf..5e0ccc9 100644 --- a/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/IgnitePdsPartitionFilesDestroyTest.java +++ b/modules/core/src/test/java/org/apache/ignite/internal/processors/cache/persistence/IgnitePdsPartitionFilesDestroyTest.java @@ -19,6 +19,7 @@ package org.apache.ignite.internal.processors.cache.persistence; import java.io.File; import java.io.IOException; import java.nio.file.OpenOption; +import java.util.List; import org.apache.ignite.Ignite; import org.apache.ignite.IgniteCache; import org.apache.ignite.IgniteCheckedException; @@ -333,6 +334,33 @@ public class IgnitePdsPartitionFilesDestroyTest extends GridCommonAbstractTest { } /** + * Test destroy when partition files are empty and there are no pages for checkpoint. + * + * @throws Exception If failed. + */ + public void testDestroyWhenPartitionsAreEmpty() throws Exception { + IgniteEx crd = (IgniteEx) startGrids(2); + + crd.cluster().active(true); + + forceCheckpoint(); + + // Evict arbitrary partition. + List<GridDhtLocalPartition> parts = crd.cachex(CACHE).context().topology().localPartitions(); + for (GridDhtLocalPartition part : parts) + if (part.state() != GridDhtPartitionState.EVICTED) { + part.rent(false).get(); + + break; + } + + // This checkpoint has no pages to write, but has one partition file to destroy. + forceCheckpoint(crd); + + checkPartitionFiles(crd, false); + } + + /** * If {@code exists} is {@code true}, checks that all partition files exist * if partition has state EVICTED. *
