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);
 

Reply via email to