This is an automated email from the ASF dual-hosted git repository. mmiller pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/main by this push: new 0d5ea4b Prevent deleted tablets from being flushed (#1899) 0d5ea4b is described below commit 0d5ea4b51e5f781c35280789d281817d4bb730ee Author: Mike Miller <mmil...@apache.org> AuthorDate: Fri Feb 12 12:13:46 2021 -0500 Prevent deleted tablets from being flushed (#1899) * Add check to LargestFirstMemoryManager.tabletsToMinorCompact() to not pick tablets from a table being deleted * Remove deleted tablet from memory reports in TabletServerResourceManager so it won't keep trying to flush delete tablets when they are large * Created isBeingDeleted() in Tablet for checking * The CleanUp step of deletes will wait until all tablets of a tablet are unassigned. This will stop the memory mgr from flushing if the table is being deleted, allowing it to be unassigned faster. * Added debug to Tablet.completeClose() for better insight when waiting * Updated LargestFirstMemoryManagerTest to test tablets being deleted * Added 1 min timeout to LargestFirstMemoryManagerTest in hopes of helping with timing issues --- .../tserver/TabletServerResourceManager.java | 4 +-- .../tserver/memory/LargestFirstMemoryManager.java | 17 ++++++---- .../org/apache/accumulo/tserver/tablet/Tablet.java | 13 +++++++- .../memory/LargestFirstMemoryManagerTest.java | 38 +++++++++++++++++----- 4 files changed, 54 insertions(+), 18 deletions(-) diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java index f011ea5..300275e 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServerResourceManager.java @@ -557,13 +557,13 @@ public class TabletServerResourceManager { } Tablet tablet = tabletReport.getTablet(); if (!tablet.initiateMinorCompaction(MinorCompactionReason.SYSTEM)) { - if (tablet.isClosed()) { + if (tablet.isClosed() || tablet.isBeingDeleted()) { // attempt to remove it from the current reports if still there synchronized (tabletReports) { TabletMemoryReport latestReport = tabletReports.remove(keyExtent); if (latestReport != null) { if (latestReport.getTablet() == tablet) { - log.debug("Cleaned up report for closed tablet {}", keyExtent); + log.debug("Cleaned up report for closed/deleted tablet {}", keyExtent); } else { // different tablet instance => put it back tabletReports.put(keyExtent, latestReport); diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/memory/LargestFirstMemoryManager.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/memory/LargestFirstMemoryManager.java index 7c242b0..c10467c 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/memory/LargestFirstMemoryManager.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/memory/LargestFirstMemoryManager.java @@ -28,6 +28,7 @@ import org.apache.accumulo.core.client.TableNotFoundException; import org.apache.accumulo.core.conf.Property; import org.apache.accumulo.core.data.TableId; import org.apache.accumulo.core.dataImpl.KeyExtent; +import org.apache.accumulo.core.manager.state.tables.TableState; import org.apache.accumulo.server.ServerContext; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -147,6 +148,10 @@ public class LargestFirstMemoryManager { return context.getTableConfiguration(tableId) != null; } + protected boolean tableBeingDeleted(TableId tableId) { + return context.getTableManager().getTableState(tableId) == TableState.DELETING; + } + public List<KeyExtent> tabletsToMinorCompact(List<TabletMemoryReport> tablets) { if (maxMemory < 0) throw new IllegalStateException( @@ -167,9 +172,10 @@ public class LargestFirstMemoryManager { // find the largest and most idle tablets for (TabletMemoryReport ts : tablets) { + KeyExtent tablet = ts.getExtent(); // Make sure that the table still exists - if (!tableExists(ts.getExtent().tableId())) { - log.trace("Ignoring extent for deleted table: {}", ts.getExtent()); + if (!tableExists(tablet.tableId()) || tableBeingDeleted(tablet.tableId())) { + log.trace("Ignoring extent for deleted table: {}", tablet); continue; } @@ -179,17 +185,16 @@ public class LargestFirstMemoryManager { final long timeMemoryLoad = timeMemoryLoad(memTabletSize, idleTime); ingestMemory += memTabletSize; if (minorCompactingSize == 0 && memTabletSize > 0) { - TabletInfo tabletInfo = - new TabletInfo(ts.getExtent(), memTabletSize, idleTime, timeMemoryLoad); + TabletInfo tabletInfo = new TabletInfo(tablet, memTabletSize, idleTime, timeMemoryLoad); try { // If the table was deleted, getMinCIdleThreshold will throw an exception - if (idleTime > getMinCIdleThreshold(ts.getExtent())) { + if (idleTime > getMinCIdleThreshold(tablet)) { largestIdleMemTablets.put(timeMemoryLoad, tabletInfo); } } catch (IllegalArgumentException e) { Throwable cause = e.getCause(); if (cause != null && cause instanceof TableNotFoundException) { - log.trace("Ignoring extent for deleted table: {}", ts.getExtent()); + log.trace("Ignoring extent for deleted table: {}", tablet); // The table might have been deleted during the iteration of the tablets // We just want to eat this exception, do nothing with this tablet, and continue diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java index 8787f7f..9c698e6 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java @@ -68,6 +68,7 @@ import org.apache.accumulo.core.iterators.SortedKeyValueIterator; import org.apache.accumulo.core.iterators.YieldCallback; import org.apache.accumulo.core.iteratorsImpl.system.SourceSwitchingIterator; import org.apache.accumulo.core.logging.TabletLogger; +import org.apache.accumulo.core.manager.state.tables.TableState; import org.apache.accumulo.core.master.thrift.BulkImportState; import org.apache.accumulo.core.metadata.MetadataTable; import org.apache.accumulo.core.metadata.StoredTabletFile; @@ -879,6 +880,10 @@ public class Tablet { if (isClosed()) { return false; } + if (isBeingDeleted()) { + log.debug("Table {} is being deleted so don't flush {}", extent.tableId(), extent); + return false; + } // get the flush id before the new memmap is made available for write long flushId; @@ -1259,6 +1264,8 @@ public class Tablet { // wait for reads and writes to complete while (writesInProgress > 0 || !activeScans.isEmpty()) { try { + log.debug("Waiting to completeClose for {}. {} writes {} scans", extent, writesInProgress, + activeScans.size()); this.wait(50); } catch (InterruptedException e) { log.error(e.toString()); @@ -1576,6 +1583,10 @@ public class Tablet { return localCS == CloseState.CLOSED || localCS == CloseState.COMPLETE; } + public boolean isBeingDeleted() { + return context.getTableManager().getTableState(extent.tableId()) == TableState.DELETING; + } + public boolean isCloseComplete() { return closeState == CloseState.COMPLETE; } @@ -1899,8 +1910,8 @@ public class Tablet { if (reason != null) { // initiate and log outside of tablet lock - initiateMinorCompaction(MinorCompactionReason.SYSTEM); log.debug("Initiating minor compaction for {} because {}", getExtent(), reason); + initiateMinorCompaction(MinorCompactionReason.SYSTEM); } } diff --git a/server/tserver/src/test/java/org/apache/accumulo/tserver/memory/LargestFirstMemoryManagerTest.java b/server/tserver/src/test/java/org/apache/accumulo/tserver/memory/LargestFirstMemoryManagerTest.java index c8b4335..9e8d089 100644 --- a/server/tserver/src/test/java/org/apache/accumulo/tserver/memory/LargestFirstMemoryManagerTest.java +++ b/server/tserver/src/test/java/org/apache/accumulo/tserver/memory/LargestFirstMemoryManagerTest.java @@ -25,7 +25,7 @@ import static org.junit.Assert.assertEquals; import java.util.Arrays; import java.util.List; -import java.util.function.Function; +import java.util.function.Predicate; import org.apache.accumulo.core.conf.AccumuloConfiguration; import org.apache.accumulo.core.conf.Property; @@ -34,9 +34,13 @@ import org.apache.accumulo.core.dataImpl.KeyExtent; import org.apache.accumulo.server.ServerContext; import org.apache.hadoop.io.Text; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.Timeout; public class LargestFirstMemoryManagerTest { + @Rule + public Timeout timeout = Timeout.seconds(60); private static final long ZERO = System.currentTimeMillis(); private static final long LATER = ZERO + 20 * 60 * 1000; @@ -171,17 +175,20 @@ public class LargestFirstMemoryManagerTest { @Test public void testDeletedTable() { final String deletedTableId = "1"; - Function<TableId,Boolean> existenceCheck = + final String beingDeleted = "2"; + Predicate<TableId> existenceCheck = tableId -> !deletedTableId.contentEquals(tableId.canonical()); + Predicate<TableId> deletingCheck = tableId -> beingDeleted.contentEquals(tableId.canonical()); LargestFirstMemoryManagerWithExistenceCheck mgr = - new LargestFirstMemoryManagerWithExistenceCheck(existenceCheck); + new LargestFirstMemoryManagerWithExistenceCheck(existenceCheck, deletingCheck); mgr.init(context); List<KeyExtent> tabletsToMinorCompact; // one tablet is really big and the other is for a nonexistent table - KeyExtent extent = new KeyExtent(TableId.of("2"), new Text("j"), null); - tabletsToMinorCompact = mgr - .tabletsToMinorCompact(tablets(t(extent, ZERO, ONE_GIG, 0), t(k("j"), ZERO, ONE_GIG, 0))); + KeyExtent extent = new KeyExtent(TableId.of("3"), new Text("j"), null); + KeyExtent extent2 = new KeyExtent(TableId.of("2"), new Text("j"), null); + tabletsToMinorCompact = mgr.tabletsToMinorCompact(tablets(t(extent, ZERO, ONE_GIG, 0), + t(extent2, ZERO, ONE_GIG, 0), t(k("j"), ZERO, ONE_GIG, 0))); assertEquals(1, tabletsToMinorCompact.size()); assertEquals(extent, tabletsToMinorCompact.get(0)); } @@ -204,21 +211,34 @@ public class LargestFirstMemoryManagerTest { protected boolean tableExists(TableId tableId) { return true; } + + @Override + protected boolean tableBeingDeleted(TableId tableId) { + return false; + } } private static class LargestFirstMemoryManagerWithExistenceCheck extends LargestFirstMemoryManagerUnderTest { - Function<TableId,Boolean> existenceCheck; + Predicate<TableId> existenceCheck; + Predicate<TableId> deletingCheck; - public LargestFirstMemoryManagerWithExistenceCheck(Function<TableId,Boolean> existenceCheck) { + public LargestFirstMemoryManagerWithExistenceCheck(Predicate<TableId> existenceCheck, + Predicate<TableId> deletingCheck) { super(); this.existenceCheck = existenceCheck; + this.deletingCheck = deletingCheck; } @Override protected boolean tableExists(TableId tableId) { - return existenceCheck.apply(tableId); + return existenceCheck.test(tableId); + } + + @Override + protected boolean tableBeingDeleted(TableId tableId) { + return deletingCheck.test(tableId); } }