This is an automated email from the ASF dual-hosted git repository.

cshannon 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 e68fa4ac22 Change getFirstKey/getLastKey to getFirstRow/getLastRow 
(#3830)
e68fa4ac22 is described below

commit e68fa4ac2243094c64451e06c554653706a37b64
Author: Christopher L. Shannon <christopher.l.shan...@gmail.com>
AuthorDate: Sat Oct 14 10:59:05 2023 -0400

    Change getFirstKey/getLastKey to getFirstRow/getLastRow (#3830)
    
    This commit changes FileSKVIterator getFirstKey and getLastKey methods
    to return the first and last row instead. These methods are only used
    for splits and in that case only the row is relevant. This was done
    because fenced RFiles will only return the row portion of a key when
    these methods are called (if the fencing is being enforced) so this
    keeps the API consistent.
---
 .../accumulo/core/file/BloomFilterLayer.java       |  8 +--
 .../apache/accumulo/core/file/FileSKVIterator.java |  6 +--
 .../core/file/rfile/MultiIndexIterator.java        |  5 +-
 .../org/apache/accumulo/core/file/rfile/RFile.java | 61 +++++++++++-----------
 .../iteratorsImpl/system/SequenceFileIterator.java |  5 +-
 .../core/file/rfile/AbstractRFileTest.java         |  8 +--
 .../core/file/rfile/MultiThreadedRFileTest.java    |  8 +--
 .../apache/accumulo/core/file/rfile/RFileTest.java | 14 ++---
 .../org/apache/accumulo/server/util/FileUtil.java  | 33 ++++++------
 .../apache/accumulo/server/util/FileInfoTest.java  | 12 ++---
 .../org/apache/accumulo/tserver/tablet/Tablet.java |  3 +-
 11 files changed, 82 insertions(+), 81 deletions(-)

diff --git 
a/core/src/main/java/org/apache/accumulo/core/file/BloomFilterLayer.java 
b/core/src/main/java/org/apache/accumulo/core/file/BloomFilterLayer.java
index 92939b5c21..5fc9a5a878 100644
--- a/core/src/main/java/org/apache/accumulo/core/file/BloomFilterLayer.java
+++ b/core/src/main/java/org/apache/accumulo/core/file/BloomFilterLayer.java
@@ -387,13 +387,13 @@ public class BloomFilterLayer {
     }
 
     @Override
-    public org.apache.accumulo.core.data.Key getFirstKey() throws IOException {
-      return reader.getFirstKey();
+    public Text getFirstRow() throws IOException {
+      return reader.getFirstRow();
     }
 
     @Override
-    public org.apache.accumulo.core.data.Key getLastKey() throws IOException {
-      return reader.getLastKey();
+    public Text getLastRow() throws IOException {
+      return reader.getLastRow();
     }
 
     @Override
diff --git 
a/core/src/main/java/org/apache/accumulo/core/file/FileSKVIterator.java 
b/core/src/main/java/org/apache/accumulo/core/file/FileSKVIterator.java
index 32ee710cb1..65c0c0bd42 100644
--- a/core/src/main/java/org/apache/accumulo/core/file/FileSKVIterator.java
+++ b/core/src/main/java/org/apache/accumulo/core/file/FileSKVIterator.java
@@ -21,15 +21,15 @@ package org.apache.accumulo.core.file;
 import java.io.DataInputStream;
 import java.io.IOException;
 
-import org.apache.accumulo.core.data.Key;
 import org.apache.accumulo.core.file.blockfile.impl.CacheProvider;
 import org.apache.accumulo.core.iteratorsImpl.system.InterruptibleIterator;
 import org.apache.accumulo.core.sample.impl.SamplerConfigurationImpl;
+import org.apache.hadoop.io.Text;
 
 public interface FileSKVIterator extends InterruptibleIterator, AutoCloseable {
-  Key getFirstKey() throws IOException;
+  Text getFirstRow() throws IOException;
 
-  Key getLastKey() throws IOException;
+  Text getLastRow() throws IOException;
 
   DataInputStream getMetaStore(String name) throws IOException, 
NoSuchMetaStoreException;
 
diff --git 
a/core/src/main/java/org/apache/accumulo/core/file/rfile/MultiIndexIterator.java
 
b/core/src/main/java/org/apache/accumulo/core/file/rfile/MultiIndexIterator.java
index b11d100359..d3a794a362 100644
--- 
a/core/src/main/java/org/apache/accumulo/core/file/rfile/MultiIndexIterator.java
+++ 
b/core/src/main/java/org/apache/accumulo/core/file/rfile/MultiIndexIterator.java
@@ -37,6 +37,7 @@ import org.apache.accumulo.core.iterators.IteratorEnvironment;
 import org.apache.accumulo.core.iterators.SortedKeyValueIterator;
 import org.apache.accumulo.core.iteratorsImpl.system.HeapIterator;
 import org.apache.accumulo.core.sample.impl.SamplerConfigurationImpl;
+import org.apache.hadoop.io.Text;
 
 class MultiIndexIterator extends HeapIterator implements FileSKVIterator {
 
@@ -80,12 +81,12 @@ class MultiIndexIterator extends HeapIterator implements 
FileSKVIterator {
   }
 
   @Override
-  public Key getFirstKey() throws IOException {
+  public Text getFirstRow() throws IOException {
     throw new UnsupportedOperationException();
   }
 
   @Override
-  public Key getLastKey() throws IOException {
+  public Text getLastRow() throws IOException {
     throw new UnsupportedOperationException();
   }
 
diff --git a/core/src/main/java/org/apache/accumulo/core/file/rfile/RFile.java 
b/core/src/main/java/org/apache/accumulo/core/file/rfile/RFile.java
index caaae410a2..c99d137b22 100644
--- a/core/src/main/java/org/apache/accumulo/core/file/rfile/RFile.java
+++ b/core/src/main/java/org/apache/accumulo/core/file/rfile/RFile.java
@@ -79,6 +79,7 @@ import 
org.apache.accumulo.core.sample.impl.SamplerConfigurationImpl;
 import org.apache.accumulo.core.util.LocalityGroupUtil;
 import org.apache.accumulo.core.util.MutableByteSequence;
 import org.apache.commons.lang3.mutable.MutableLong;
+import org.apache.hadoop.io.Text;
 import org.apache.hadoop.io.Writable;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -1106,16 +1107,16 @@ public class RFile {
     }
 
     @Override
-    public Key getFirstKey() {
-      return firstKey;
+    public Text getFirstRow() {
+      return firstKey != null ? firstKey.getRow() : null;
     }
 
     @Override
-    public Key getLastKey() {
+    public Text getLastRow() {
       if (index.size() == 0) {
         return null;
       }
-      return index.getLastKey();
+      return index.getLastKey().getRow();
     }
 
     @Override
@@ -1340,47 +1341,47 @@ public class RFile {
     }
 
     @Override
-    public Key getFirstKey() throws IOException {
+    public Text getFirstRow() throws IOException {
       if (currentReaders.length == 0) {
         return null;
       }
 
-      Key minKey = null;
+      Text minRow = null;
 
       for (LocalityGroupReader currentReader : currentReaders) {
-        if (minKey == null) {
-          minKey = currentReader.getFirstKey();
+        if (minRow == null) {
+          minRow = currentReader.getFirstRow();
         } else {
-          Key firstKey = currentReader.getFirstKey();
-          if (firstKey != null && firstKey.compareTo(minKey) < 0) {
-            minKey = firstKey;
+          Text firstRow = currentReader.getFirstRow();
+          if (firstRow != null && firstRow.compareTo(minRow) < 0) {
+            minRow = firstRow;
           }
         }
       }
 
-      return minKey;
+      return minRow;
     }
 
     @Override
-    public Key getLastKey() throws IOException {
+    public Text getLastRow() throws IOException {
       if (currentReaders.length == 0) {
         return null;
       }
 
-      Key maxKey = null;
+      Text maxRow = null;
 
       for (LocalityGroupReader currentReader : currentReaders) {
-        if (maxKey == null) {
-          maxKey = currentReader.getLastKey();
+        if (maxRow == null) {
+          maxRow = currentReader.getLastRow();
         } else {
-          Key lastKey = currentReader.getLastKey();
-          if (lastKey != null && lastKey.compareTo(maxKey) > 0) {
-            maxKey = lastKey;
+          Text lastRow = currentReader.getLastRow();
+          if (lastRow != null && lastRow.compareTo(maxRow) > 0) {
+            maxRow = lastRow;
           }
         }
       }
 
-      return maxKey;
+      return maxRow;
     }
 
     @Override
@@ -1617,22 +1618,22 @@ public class RFile {
     }
 
     @Override
-    public Key getFirstKey() throws IOException {
-      var rfk = reader.getFirstKey();
-      if (fence.beforeStartKey(rfk)) {
-        return fencedStartKey;
+    public Text getFirstRow() throws IOException {
+      var row = reader.getFirstRow();
+      if (row != null && fence.beforeStartKey(new Key(row))) {
+        return fencedStartKey.getRow();
       } else {
-        return rfk;
+        return row;
       }
     }
 
     @Override
-    public Key getLastKey() throws IOException {
-      var rlk = reader.getLastKey();
-      if (fence.afterEndKey(rlk)) {
-        return fencedEndKey.get();
+    public Text getLastRow() throws IOException {
+      var row = reader.getLastRow();
+      if (row != null && fence.afterEndKey(new Key(row))) {
+        return fencedEndKey.get().getRow();
       } else {
-        return rlk;
+        return row;
       }
     }
 
diff --git 
a/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/system/SequenceFileIterator.java
 
b/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/system/SequenceFileIterator.java
index 44d0d600b3..e1b9fe3897 100644
--- 
a/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/system/SequenceFileIterator.java
+++ 
b/core/src/main/java/org/apache/accumulo/core/iteratorsImpl/system/SequenceFileIterator.java
@@ -35,6 +35,7 @@ import 
org.apache.accumulo.core.iterators.SortedKeyValueIterator;
 import org.apache.accumulo.core.sample.impl.SamplerConfigurationImpl;
 import org.apache.hadoop.io.SequenceFile;
 import org.apache.hadoop.io.SequenceFile.Reader;
+import org.apache.hadoop.io.Text;
 
 public class SequenceFileIterator implements FileSKVIterator {
 
@@ -116,12 +117,12 @@ public class SequenceFileIterator implements 
FileSKVIterator {
   }
 
   @Override
-  public Key getFirstKey() throws IOException {
+  public Text getFirstRow() throws IOException {
     throw new UnsupportedOperationException("getFirstKey() not supported");
   }
 
   @Override
-  public Key getLastKey() throws IOException {
+  public Text getLastRow() throws IOException {
     throw new UnsupportedOperationException("getLastKey() not supported");
   }
 
diff --git 
a/core/src/test/java/org/apache/accumulo/core/file/rfile/AbstractRFileTest.java 
b/core/src/test/java/org/apache/accumulo/core/file/rfile/AbstractRFileTest.java
index 5fa0a1b7a0..a220407b92 100644
--- 
a/core/src/test/java/org/apache/accumulo/core/file/rfile/AbstractRFileTest.java
+++ 
b/core/src/test/java/org/apache/accumulo/core/file/rfile/AbstractRFileTest.java
@@ -204,9 +204,9 @@ public abstract class AbstractRFileTest {
     if (indexIter.hasTop()) {
       Key lastKey = new Key(indexIter.getTopKey());
 
-      if (reader.getFirstKey().compareTo(lastKey) > 0) {
+      if (reader.getFirstRow().compareTo(lastKey.getRow()) > 0) {
         throw new IllegalStateException(
-            "First key out of order " + reader.getFirstKey() + " " + lastKey);
+            "First key out of order " + reader.getFirstRow() + " " + lastKey);
       }
 
       indexIter.next();
@@ -222,9 +222,9 @@ public abstract class AbstractRFileTest {
 
       }
 
-      if (!reader.getLastKey().equals(lastKey)) {
+      if (!reader.getLastRow().equals(lastKey.getRow())) {
         throw new IllegalStateException(
-            "Last key out of order " + reader.getLastKey() + " " + lastKey);
+            "Last key out of order " + reader.getLastRow() + " " + lastKey);
       }
     }
   }
diff --git 
a/core/src/test/java/org/apache/accumulo/core/file/rfile/MultiThreadedRFileTest.java
 
b/core/src/test/java/org/apache/accumulo/core/file/rfile/MultiThreadedRFileTest.java
index 88a6e67e9d..8c66bb6ead 100644
--- 
a/core/src/test/java/org/apache/accumulo/core/file/rfile/MultiThreadedRFileTest.java
+++ 
b/core/src/test/java/org/apache/accumulo/core/file/rfile/MultiThreadedRFileTest.java
@@ -80,9 +80,9 @@ public class MultiThreadedRFileTest {
     if (indexIter.hasTop()) {
       Key lastKey = new Key(indexIter.getTopKey());
 
-      if (reader.getFirstKey().compareTo(lastKey) > 0) {
+      if (reader.getFirstRow().compareTo(lastKey.getRow()) > 0) {
         throw new IllegalStateException(
-            "First key out of order " + reader.getFirstKey() + " " + lastKey);
+            "First key out of order " + reader.getFirstRow() + " " + lastKey);
       }
 
       indexIter.next();
@@ -98,9 +98,9 @@ public class MultiThreadedRFileTest {
 
       }
 
-      if (!reader.getLastKey().equals(lastKey)) {
+      if (!reader.getLastRow().equals(lastKey.getRow())) {
         throw new IllegalStateException(
-            "Last key out of order " + reader.getLastKey() + " " + lastKey);
+            "Last key out of order " + reader.getLastRow() + " " + lastKey);
       }
     }
   }
diff --git 
a/core/src/test/java/org/apache/accumulo/core/file/rfile/RFileTest.java 
b/core/src/test/java/org/apache/accumulo/core/file/rfile/RFileTest.java
index 18c05488b1..0ac78c1752 100644
--- a/core/src/test/java/org/apache/accumulo/core/file/rfile/RFileTest.java
+++ b/core/src/test/java/org/apache/accumulo/core/file/rfile/RFileTest.java
@@ -215,7 +215,7 @@ public class RFileTest extends AbstractRFileTest {
     trf.iter.seek(new Range((Key) null, null), EMPTY_COL_FAMS, false);
     assertFalse(trf.iter.hasTop());
 
-    assertNull(trf.reader.getLastKey());
+    assertNull(trf.reader.getLastRow());
 
     trf.closeReader();
   }
@@ -252,7 +252,7 @@ public class RFileTest extends AbstractRFileTest {
     trf.iter.next();
     assertFalse(trf.iter.hasTop());
 
-    assertEquals(newKey("r1", "cf1", "cq1", "L1", 55), 
trf.reader.getLastKey());
+    assertEquals(new Text("r1"), trf.reader.getLastRow());
 
     trf.closeReader();
   }
@@ -388,7 +388,7 @@ public class RFileTest extends AbstractRFileTest {
       }
     }
 
-    assertEquals(expectedKeys.get(expectedKeys.size() - 1), 
trf.reader.getLastKey());
+    assertEquals(expectedKeys.get(expectedKeys.size() - 1).getRow(), 
trf.reader.getLastRow());
 
     // test seeking to random location and reading all data from that point
     // there was an off by one bug with this in the transient index
@@ -464,7 +464,7 @@ public class RFileTest extends AbstractRFileTest {
     assertEquals(newKey("r1", "cf1", "cq1", "L1", 55), trf.iter.getTopKey());
     assertEquals(newValue("foo1"), trf.iter.getTopValue());
 
-    assertEquals(newKey("r1", "cf1", "cq4", "L1", 56), 
trf.reader.getLastKey());
+    assertEquals(new Text("r1"), trf.reader.getLastRow());
 
     trf.closeReader();
   }
@@ -497,7 +497,7 @@ public class RFileTest extends AbstractRFileTest {
       assertFalse(trf.iter.hasTop());
     }
 
-    assertEquals(newKey(formatString("r_", 499), "cf1", "cq1", "L1", 55), 
trf.reader.getLastKey());
+    assertEquals(new Text(formatString("r_", 499)), trf.reader.getLastRow());
 
     trf.closeReader();
   }
@@ -563,7 +563,7 @@ public class RFileTest extends AbstractRFileTest {
         newKey(formatString("r_", 2), "cf1", "cq1", "L1", 55), false), 
EMPTY_COL_FAMS, false);
     assertFalse(trf.iter.hasTop());
 
-    assertEquals(newKey(formatString("r_", 49), "cf1", "cq1", "L1", 55), 
trf.reader.getLastKey());
+    assertEquals(new Text(formatString("r_", 49)), trf.reader.getLastRow());
 
     trf.reader.close();
   }
@@ -2181,7 +2181,7 @@ public class RFileTest extends AbstractRFileTest {
     testRfile.iter.seek(new Range((Key) null, null), EMPTY_COL_FAMS, false);
     assertTrue(testRfile.iter.hasTop());
 
-    assertNotNull(testRfile.reader.getLastKey());
+    assertNotNull(testRfile.reader.getLastRow());
 
     testRfile.closeReader();
 
diff --git 
a/server/base/src/main/java/org/apache/accumulo/server/util/FileUtil.java 
b/server/base/src/main/java/org/apache/accumulo/server/util/FileUtil.java
index 31b7dd746f..a600dff94b 100644
--- a/server/base/src/main/java/org/apache/accumulo/server/util/FileUtil.java
+++ b/server/base/src/main/java/org/apache/accumulo/server/util/FileUtil.java
@@ -54,27 +54,26 @@ import org.apache.accumulo.server.fs.VolumeManager;
 import org.apache.hadoop.fs.FileSystem;
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.io.Text;
-import org.apache.hadoop.io.WritableComparable;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 public class FileUtil {
 
   public static class FileInfo {
-    Key firstKey = new Key();
-    Key lastKey = new Key();
+    final Text firstRow;
+    final Text lastRow;
 
-    public FileInfo(Key firstKey, Key lastKey) {
-      this.firstKey = firstKey;
-      this.lastKey = lastKey;
+    public FileInfo(Text firstRow, Text lastRow) {
+      this.firstRow = firstRow;
+      this.lastRow = lastRow;
     }
 
     public Text getFirstRow() {
-      return firstKey.getRow();
+      return firstRow;
     }
 
     public Text getLastRow() {
-      return lastKey.getRow();
+      return lastRow;
     }
   }
 
@@ -500,9 +499,9 @@ public class FileUtil {
             .forFile(dataFile, ns, ns.getConf(), tableConf.getCryptoService())
             .withTableConfiguration(tableConf).build();
 
-        Key firstKey = reader.getFirstKey();
-        if (firstKey != null) {
-          dataFilesInfo.put(dataFile, new FileInfo(firstKey, 
reader.getLastKey()));
+        Text firstRow = reader.getFirstRow();
+        if (firstRow != null) {
+          dataFilesInfo.put(dataFile, new FileInfo(firstRow, 
reader.getLastRow()));
         }
 
       } catch (IOException ioe) {
@@ -527,10 +526,10 @@ public class FileUtil {
     return dataFilesInfo;
   }
 
-  public static <T extends TabletFile> WritableComparable<Key> 
findLastKey(ServerContext context,
+  public static <T extends TabletFile> Text findLastRow(ServerContext context,
       TableConfiguration tableConf, Collection<T> dataFiles) throws 
IOException {
 
-    Key lastKey = null;
+    Text lastRow = null;
 
     for (T file : dataFiles) {
       FileSystem ns = 
context.getVolumeManager().getFileSystemByPath(file.getPath());
@@ -544,10 +543,10 @@ public class FileUtil {
           continue;
         }
 
-        Key key = reader.getLastKey();
+        Text row = reader.getLastRow();
 
-        if (lastKey == null || key.compareTo(lastKey) > 0) {
-          lastKey = key;
+        if (lastRow == null || row.compareTo(lastRow) > 0) {
+          lastRow = row;
         }
       } finally {
         try {
@@ -560,7 +559,7 @@ public class FileUtil {
       }
     }
 
-    return lastKey;
+    return lastRow;
 
   }
 
diff --git 
a/server/base/src/test/java/org/apache/accumulo/server/util/FileInfoTest.java 
b/server/base/src/test/java/org/apache/accumulo/server/util/FileInfoTest.java
index d38b5a5fa3..a65d776e7b 100644
--- 
a/server/base/src/test/java/org/apache/accumulo/server/util/FileInfoTest.java
+++ 
b/server/base/src/test/java/org/apache/accumulo/server/util/FileInfoTest.java
@@ -20,21 +20,21 @@ package org.apache.accumulo.server.util;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
-import org.apache.accumulo.core.data.Key;
 import org.apache.accumulo.server.util.FileUtil.FileInfo;
+import org.apache.hadoop.io.Text;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
 public class FileInfoTest {
-  private Key key1;
-  private Key key2;
+  private Text row1;
+  private Text row2;
   private FileInfo info;
 
   @BeforeEach
   public void setUp() {
-    key1 = new Key("row1");
-    key2 = new Key("row2");
-    info = new FileInfo(key1, key2);
+    row1 = new Text("row1");
+    row2 = new Text("row2");
+    info = new FileInfo(row1, row2);
   }
 
   @Test
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 f3337a4e25..fa1f4c50a8 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
@@ -1441,8 +1441,7 @@ public class Tablet extends TabletBase {
         Text lastRow = null;
 
         if (extent.endRow() == null) {
-          Key lastKey = (Key) FileUtil.findLastKey(context, 
tableConfiguration, files);
-          lastRow = lastKey.getRow();
+          lastRow = FileUtil.findLastRow(context, tableConfiguration, files);
         }
 
         newComputation = new SplitComputations(files, midpoint, lastRow);

Reply via email to