From 90bcb465fe672ec0181bbcd8acc1d6f4f22e66be Mon Sep 17 00:00:00 2001 From: "Christopher L. Shannon" Date: Sat, 6 Apr 2024 15:20:07 -0400 Subject: [PATCH] Ensure getTabletState() in TabletMetadata does not allow a null extent (#4438) Previously the private reference for the extent inside TabletMetadata was passed to the TabletLocationState constructor inside of getTabletState(). The extent may not have been loaded at this point as it is lazy loaded by the getExtent() method and requires PREV_ROW to have been fetched. This change now uses the getter to make sure we do not inadvertently pass a null extent which is invalid. --- .../accumulo/core/metadata/TabletLocationState.java | 3 ++- .../accumulo/core/metadata/schema/TabletMetadata.java | 4 +++- .../accumulo/core/metadata/schema/TabletMetadataTest.java | 8 ++++++++ 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/apache/accumulo/core/metadata/TabletLocationState.java b/core/src/main/java/org/apache/accumulo/core/metadata/TabletLocationState.java index d0cd66300b5..0a705940273 100644 --- a/core/src/main/java/org/apache/accumulo/core/metadata/TabletLocationState.java +++ b/core/src/main/java/org/apache/accumulo/core/metadata/TabletLocationState.java @@ -22,6 +22,7 @@ import java.util.Collection; import java.util.Collections; +import java.util.Objects; import java.util.Set; import org.apache.accumulo.core.dataImpl.KeyExtent; @@ -58,7 +59,7 @@ public Text getEncodedEndRow() { public TabletLocationState(KeyExtent extent, Location future, Location current, Location last, SuspendingTServer suspend, Collection> walogs, boolean chopped) throws BadLocationStateException { - this.extent = extent; + this.extent = Objects.requireNonNull(extent); this.future = validateLocation(future, TabletMetadata.LocationType.FUTURE); this.current = validateLocation(current, TabletMetadata.LocationType.CURRENT); this.last = validateLocation(last, TabletMetadata.LocationType.LAST); 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 323f0cb1420..c1e816109af 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 @@ -373,7 +373,9 @@ public TabletState getTabletState(Set liveTServers) { future = location; } // only care about the state so don't need walogs and chopped params - var tls = new TabletLocationState(extent, future, current, last, suspend, null, false); + // Use getExtent() when passing the extent as the private reference may not have been + // initialized yet. This will also ensure PREV_ROW was fetched + var tls = new TabletLocationState(getExtent(), future, current, last, suspend, null, false); return tls.getState(liveTServers); } catch (TabletLocationState.BadLocationStateException blse) { throw new IllegalArgumentException("Error creating TabletLocationState", blse); 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 67a22ccd3f0..7d9284ba009 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 @@ -26,6 +26,7 @@ import static org.apache.accumulo.core.metadata.schema.MetadataSchema.TabletsSection.ServerColumnFamily.TIME_COLUMN; import static org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType.LAST; import static org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType.LOCATION; +import static org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType.PREV_ROW; import static org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType.SUSPEND; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -196,6 +197,13 @@ public void testLocationStates() { .put(ser1.getHostPort()); SortedMap rowMap = toRowMap(mutation); + // PREV_ROW was not fetched + final var missingPrevRow = + TabletMetadata.convertRow(rowMap.entrySet().iterator(), colsToFetch, false); + assertThrows(IllegalStateException.class, () -> missingPrevRow.getTabletState(tservers)); + + // This should now work as PREV_ROW has been included + colsToFetch = EnumSet.of(LOCATION, LAST, SUSPEND, PREV_ROW); TabletMetadata tm = TabletMetadata.convertRow(rowMap.entrySet().iterator(), colsToFetch, false); TabletState state = tm.getTabletState(tservers);