You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@accumulo.apache.org by do...@apache.org on 2023/06/26 16:55:10 UTC
[accumulo] branch elasticity updated: Adds validation that tablet does not have operation id and location at the same time (#3453)
This is an automated email from the ASF dual-hosted git repository.
domgarguilo pushed a commit to branch elasticity
in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/elasticity by this push:
new e60390d580 Adds validation that tablet does not have operation id and location at the same time (#3453)
e60390d580 is described below
commit e60390d5807ed6397a439fb9db617a19bda28e2a
Author: Dom G <do...@apache.org>
AuthorDate: Mon Jun 26 12:55:03 2023 -0400
Adds validation that tablet does not have operation id and location at the same time (#3453)
* Adds validation that tablet does not have operation id and location at the same time
* Use ToStringBuilder in TabletMetadata
---------
Co-authored-by: Keith Turner <kt...@apache.org>
---
.../core/metadata/schema/TabletMetadata.java | 71 ++++++++++++++++++----
.../core/metadata/schema/TabletMetadataTest.java | 32 +++++++++-
.../manager/state/TabletManagementIterator.java | 2 +-
.../accumulo/manager/TabletGroupWatcher.java | 5 ++
4 files changed, 95 insertions(+), 15 deletions(-)
diff --git a/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java b/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java
index f4757919f3..ca1ab452b0 100644
--- a/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java
+++ b/core/src/main/java/org/apache/accumulo/core/metadata/schema/TabletMetadata.java
@@ -75,6 +75,8 @@ import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.Se
import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.SuspendLocationColumn;
import org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.TabletColumnFamily;
import org.apache.accumulo.core.tabletserver.log.LogEntry;
+import org.apache.commons.lang3.builder.ToStringBuilder;
+import org.apache.commons.lang3.builder.ToStringStyle;
import org.apache.hadoop.io.Text;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -118,6 +120,7 @@ public class TabletMetadata {
protected boolean onDemandHostingRequested = false;
private TabletOperationId operationId;
protected boolean futureAndCurrentLocationSet = false;
+ protected boolean operationIdAndCurrentLocationSet = false;
public enum LocationType {
CURRENT, FUTURE, LAST
@@ -384,16 +387,20 @@ public class TabletMetadata {
@Override
public String toString() {
- return "TabletMetadata [tableId=" + tableId + ", prevEndRow=" + prevEndRow + ", sawPrevEndRow="
- + sawPrevEndRow + ", oldPrevEndRow=" + oldPrevEndRow + ", sawOldPrevEndRow="
- + sawOldPrevEndRow + ", endRow=" + endRow + ", location=" + location + ", files=" + files
- + ", scans=" + scans + ", loadedFiles=" + loadedFiles + ", fetchedCols=" + fetchedCols
- + ", extent=" + extent + ", last=" + last + ", suspend=" + suspend + ", dirName=" + dirName
- + ", time=" + time + ", cloned=" + cloned + ", flush=" + flush + ", logs=" + logs
- + ", compact=" + compact + ", splitRatio=" + splitRatio + ", extCompactions="
- + extCompactions + ", chopped=" + chopped + ", goal=" + goal + ", onDemandHostingRequested="
- + onDemandHostingRequested + ", operationId=" + operationId
- + ", futureAndCurrentLocationSet=" + futureAndCurrentLocationSet + "]";
+ return new ToStringBuilder(this, ToStringStyle.SHORT_PREFIX_STYLE).append("tableId", tableId)
+ .append("prevEndRow", prevEndRow).append("sawPrevEndRow", sawPrevEndRow)
+ .append("oldPrevEndRow", oldPrevEndRow).append("sawOldPrevEndRow", sawOldPrevEndRow)
+ .append("endRow", endRow).append("location", location).append("files", files)
+ .append("scans", scans).append("loadedFiles", loadedFiles)
+ .append("fetchedCols", fetchedCols).append("extent", extent).append("last", last)
+ .append("suspend", suspend).append("dirName", dirName).append("time", time)
+ .append("cloned", cloned).append("flush", flush).append("logs", logs)
+ .append("compact", compact).append("splitRatio", splitRatio)
+ .append("extCompactions", extCompactions).append("chopped", chopped).append("goal", goal)
+ .append("onDemandHostingRequested", onDemandHostingRequested)
+ .append("operationId", operationId)
+ .append("futureAndCurrentLocationSet", futureAndCurrentLocationSet)
+ .append("operationIdAndCurrentLocationSet", operationIdAndCurrentLocationSet).toString();
}
public SortedMap<Key,Value> getKeyValues() {
@@ -443,6 +450,10 @@ public class TabletMetadata {
return futureAndCurrentLocationSet;
}
+ public boolean isOperationIdAndCurrentLocationSet() {
+ return operationIdAndCurrentLocationSet;
+ }
+
@VisibleForTesting
public static <E extends Entry<Key,Value>> TabletMetadata convertRow(Iterator<E> rowIter,
EnumSet<ColumnType> fetchedColumns, boolean buildKeyValueMap, boolean suppressLocationError) {
@@ -515,7 +526,7 @@ public class TabletMetadata {
te.compact = OptionalLong.of(Long.parseLong(val));
break;
case OPID_QUAL:
- te.operationId = TabletOperationId.from(val);
+ te.setOperationIdOnce(val, suppressLocationError);
break;
}
break;
@@ -586,6 +597,15 @@ public class TabletMetadata {
return te;
}
+ /**
+ * Sets a location only once.
+ *
+ * @param val server to set for Location object
+ * @param qual session to set for Location object
+ * @param lt location type to use to construct Location object
+ * @param suppressError set to true to suppress an exception being thrown, else false
+ * @throws IllegalStateException if an operation id or location is already set
+ */
private void setLocationOnce(String val, String qual, LocationType lt, boolean suppressError) {
if (location != null) {
if (!suppressError) {
@@ -594,9 +614,38 @@ public class TabletMetadata {
}
futureAndCurrentLocationSet = true;
}
+ if (operationId != null) {
+ if (!suppressError) {
+ throw new IllegalStateException(
+ "Attempted to set location for tablet with an operation id. table ID: " + tableId
+ + " endrow: " + endRow + " -- operation id: " + operationId);
+ }
+ operationIdAndCurrentLocationSet = true;
+ }
location = new Location(val, qual, lt);
}
+ /**
+ * Sets an operation ID only once.
+ *
+ * @param val operation id to set
+ * @param suppressError set to true to suppress an exception being thrown, else false
+ * @throws IllegalStateException if an operation id or location is already set
+ */
+ private void setOperationIdOnce(String val, boolean suppressError) {
+ Preconditions.checkState(operationId == null);
+ // make sure there is not already a current location set
+ if (location != null) {
+ if (!suppressError) {
+ throw new IllegalStateException(
+ "Attempted to set operation id for tablet with current location. table ID: " + tableId
+ + " endrow: " + endRow + " -- location: " + location);
+ }
+ operationIdAndCurrentLocationSet = true;
+ }
+ operationId = TabletOperationId.from(val);
+ }
+
@VisibleForTesting
static TabletMetadata create(String id, String prevEndRow, String endRow) {
TabletMetadata te = new TabletMetadata();
diff --git a/core/src/test/java/org/apache/accumulo/core/metadata/schema/TabletMetadataTest.java b/core/src/test/java/org/apache/accumulo/core/metadata/schema/TabletMetadataTest.java
index 43e6a0e5e0..d41d835183 100644
--- a/core/src/test/java/org/apache/accumulo/core/metadata/schema/TabletMetadataTest.java
+++ b/core/src/test/java/org/apache/accumulo/core/metadata/schema/TabletMetadataTest.java
@@ -19,9 +19,11 @@
package org.apache.accumulo.core.metadata.schema;
import static java.util.stream.Collectors.toSet;
+import static org.apache.accumulo.core.fate.FateTxId.formatTid;
import static org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ServerColumnFamily.COMPACT_COLUMN;
import static org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ServerColumnFamily.DIRECTORY_COLUMN;
import static org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ServerColumnFamily.FLUSH_COLUMN;
+import static org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ServerColumnFamily.OPID_QUAL;
import static org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ServerColumnFamily.TIME_COLUMN;
import static org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType.HOSTING_GOAL;
import static org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType.HOSTING_REQUESTED;
@@ -46,7 +48,6 @@ import org.apache.accumulo.core.data.Mutation;
import org.apache.accumulo.core.data.TableId;
import org.apache.accumulo.core.data.Value;
import org.apache.accumulo.core.dataImpl.KeyExtent;
-import org.apache.accumulo.core.fate.FateTxId;
import org.apache.accumulo.core.metadata.StoredTabletFile;
import org.apache.accumulo.core.metadata.SuspendingTServer;
import org.apache.accumulo.core.metadata.TServerInstance;
@@ -83,8 +84,8 @@ public class TabletMetadataTest {
String bf1 = "hdfs://nn1/acc/tables/1/t-0001/bf1";
String bf2 = "hdfs://nn1/acc/tables/1/t-0001/bf2";
- mutation.at().family(BulkFileColumnFamily.NAME).qualifier(bf1).put(FateTxId.formatTid(56));
- mutation.at().family(BulkFileColumnFamily.NAME).qualifier(bf2).put(FateTxId.formatTid(59));
+ mutation.at().family(BulkFileColumnFamily.NAME).qualifier(bf1).put(formatTid(56));
+ mutation.at().family(BulkFileColumnFamily.NAME).qualifier(bf2).put(formatTid(59));
mutation.at().family(ClonedColumnFamily.NAME).qualifier("").put("OK");
@@ -182,6 +183,31 @@ public class TabletMetadataTest {
assertTrue(tm.isFutureAndCurrentLocationSet());
}
+ @Test
+ public void testTableOpAndCurrentLocation() {
+ final long tableId = 5L;
+ final KeyExtent extent =
+ new KeyExtent(TableId.of(Long.toString(tableId)), new Text("df"), new Text("da"));
+
+ Mutation mutation = TabletColumnFamily.createPrevRowMutation(extent);
+
+ // set a tablet operation as well as future location
+ mutation.at().family(MetadataSchema.TabletsSection.ServerColumnFamily.STR_NAME)
+ .qualifier(OPID_QUAL).put(TabletOperationType.DELETING + ":" + formatTid(tableId));
+ mutation.at().family(FutureLocationColumnFamily.NAME).qualifier("s001").put("server1:8555");
+
+ SortedMap<Key,Value> rowMap = toRowMap(mutation);
+
+ assertThrows(IllegalStateException.class,
+ () -> TabletMetadata.convertRow(rowMap.entrySet().iterator(),
+ EnumSet.allOf(ColumnType.class), false, false),
+ "tablet should not have operation id and current location at the same time");
+
+ TabletMetadata tm = TabletMetadata.convertRow(rowMap.entrySet().iterator(),
+ EnumSet.allOf(ColumnType.class), false, true);
+ assertTrue(tm.isOperationIdAndCurrentLocationSet());
+ }
+
@Test
public void testLocationStates() {
KeyExtent extent = new KeyExtent(TableId.of("5"), new Text("df"), new Text("da"));
diff --git a/server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletManagementIterator.java b/server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletManagementIterator.java
index 3af8cf9f76..4b8abe2ff5 100644
--- a/server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletManagementIterator.java
+++ b/server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletManagementIterator.java
@@ -403,7 +403,7 @@ public class TabletManagementIterator extends SkippingIterator {
private void computeTabletManagementActions(final TabletMetadata tm,
final Set<ManagementAction> reasonsToReturnThisTablet) {
- if (tm.isFutureAndCurrentLocationSet()) {
+ if (tm.isFutureAndCurrentLocationSet() || tm.isOperationIdAndCurrentLocationSet()) {
// no need to check everything, we are in a known state where we want to return everything.
reasonsToReturnThisTablet.add(ManagementAction.BAD_STATE);
return;
diff --git a/server/manager/src/main/java/org/apache/accumulo/manager/TabletGroupWatcher.java b/server/manager/src/main/java/org/apache/accumulo/manager/TabletGroupWatcher.java
index 114863d05e..04a97b752c 100644
--- a/server/manager/src/main/java/org/apache/accumulo/manager/TabletGroupWatcher.java
+++ b/server/manager/src/main/java/org/apache/accumulo/manager/TabletGroupWatcher.java
@@ -258,6 +258,11 @@ abstract class TabletGroupWatcher extends AccumuloDaemonThread {
tm.getExtent() + " is both assigned and hosted, which should never happen: " + this,
tm.getExtent().toMetaRow());
}
+ if (tm.isOperationIdAndCurrentLocationSet()) {
+ throw new BadLocationStateException(tm.getExtent()
+ + " has both operation id and current location, which should never happen: " + this,
+ tm.getExtent().toMetaRow());
+ }
final TableId tableId = tm.getTableId();
// ignore entries for tables that do not exist in zookeeper