You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@accumulo.apache.org by mm...@apache.org on 2021/02/12 17:13:59 UTC
[accumulo] branch main updated: Prevent deleted tablets from being
flushed (#1899)
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 <mm...@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);
}
}