This is an automated email from the ASF dual-hosted git repository. cshannon pushed a commit to branch 2.1 in repository https://gitbox.apache.org/repos/asf/accumulo.git
The following commit(s) were added to refs/heads/2.1 by this push: new 90bcb465fe Ensure getTabletState() in TabletMetadata does not allow a null extent (#4438) 90bcb465fe is described below commit 90bcb465fe672ec0181bbcd8acc1d6f4f22e66be Author: Christopher L. Shannon <cshan...@apache.org> AuthorDate: Sat Apr 6 15:20:07 2024 -0400 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. --- .../org/apache/accumulo/core/metadata/TabletLocationState.java | 3 ++- .../org/apache/accumulo/core/metadata/schema/TabletMetadata.java | 4 +++- .../apache/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 d0cd66300b..0a70594027 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 static java.util.Objects.requireNonNull; 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 class TabletLocationState { public TabletLocationState(KeyExtent extent, Location future, Location current, Location last, SuspendingTServer suspend, Collection<Collection<String>> 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 323f0cb142..c1e816109a 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 class TabletMetadata { 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 67a22ccd3f..7d9284ba00 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.TabletsSec 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 class TabletMetadataTest { .put(ser1.getHostPort()); SortedMap<Key,Value> 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);