HBASE-28729 Change the generic type of List in InternalScanner.next (#…

…6126)

Signed-off-by: Xin Sun <sunxin@apache.org>
(cherry picked from commit 939ab48)

hbase-examples/src/main/java/org/apache/hadoop/hbase/coprocessor/example/DelegatingInternalScanner.java

@@ -19,7 +19,7 @@
 
 import java.io.IOException;
 import java.util.List;
-import org.apache.hadoop.hbase.Cell;
+import org.apache.hadoop.hbase.ExtendedCell;
 import org.apache.hadoop.hbase.regionserver.InternalScanner;
 import org.apache.hadoop.hbase.regionserver.ScannerContext;
 import org.apache.yetus.audience.InterfaceAudience;
@@ -37,7 +37,8 @@ public DelegatingInternalScanner(InternalScanner scanner) {
   }
 
   @Override
-  public boolean next(List<Cell> result, ScannerContext scannerContext) throws IOException {
+  public boolean next(List<? super ExtendedCell> result, ScannerContext scannerContext)
+    throws IOException {
     return scanner.next(result, scannerContext);
   }
 

hbase-examples/src/main/java/org/apache/hadoop/hbase/coprocessor/example/ValueRewritingObserver.java

@@ -26,6 +26,7 @@
 import org.apache.hadoop.hbase.CellBuilderType;
 import org.apache.hadoop.hbase.CellUtil;
 import org.apache.hadoop.hbase.CoprocessorEnvironment;
+import org.apache.hadoop.hbase.ExtendedCell;
 import org.apache.hadoop.hbase.coprocessor.ObserverContext;
 import org.apache.hadoop.hbase.coprocessor.RegionCoprocessor;
 import org.apache.hadoop.hbase.coprocessor.RegionCoprocessorEnvironment;
@@ -74,10 +75,11 @@ public InternalScanner preCompact(ObserverContext<RegionCoprocessorEnvironment>
     CompactionRequest request) {
     InternalScanner modifyingScanner = new InternalScanner() {
       @Override
-      public boolean next(List<Cell> result, ScannerContext scannerContext) throws IOException {
+      public boolean next(List<? super ExtendedCell> result, ScannerContext scannerContext)
+        throws IOException {
         boolean ret = scanner.next(result, scannerContext);
         for (int i = 0; i < result.size(); i++) {
-          Cell c = result.get(i);
+          Cell c = (Cell) result.get(i);
           // Replace the Cell if the value is the one we're replacing
           if (CellUtil.isPut(c) && comparator.compare(CellUtil.cloneValue(c), sourceValue) == 0) {
             try {
@@ -90,7 +92,9 @@ public boolean next(List<Cell> result, ScannerContext scannerContext) throws IOE
               byte[] clonedValue = new byte[replacedValue.length];
               System.arraycopy(replacedValue, 0, clonedValue, 0, replacedValue.length);
               cellBuilder.setValue(clonedValue);
-              result.set(i, cellBuilder.build());
+              // all cells in HBase are ExtendedCells, so you are fine to cast it to ExtendedCell,
+              // just do not use its methods since it may change without any deprecation cycle
+              result.set(i, (ExtendedCell) cellBuilder.build());
             } finally {
               cellBuilder.clear();
             }

hbase-examples/src/main/java/org/apache/hadoop/hbase/coprocessor/example/WriteHeavyIncrementObserver.java

@@ -31,6 +31,7 @@
 import org.apache.hadoop.hbase.CellBuilderFactory;
 import org.apache.hadoop.hbase.CellBuilderType;
 import org.apache.hadoop.hbase.CellUtil;
+import org.apache.hadoop.hbase.ExtendedCell;
 import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.client.Get;
 import org.apache.hadoop.hbase.client.Increment;
@@ -99,11 +100,14 @@ private InternalScanner wrap(byte[] family, InternalScanner scanner) {
       private long sum;
 
       @Override
-      public boolean next(List<Cell> result, ScannerContext scannerContext) throws IOException {
+      public boolean next(List<? super ExtendedCell> result, ScannerContext scannerContext)
+        throws IOException {
         boolean moreRows = scanner.next(srcResult, scannerContext);
         if (srcResult.isEmpty()) {
           if (!moreRows && row != null) {
-            result.add(createCell(row, family, qualifier, timestamp, sum));
+            // all cells in HBase are ExtendedCells, so you are fine to cast it to ExtendedCell,
+            // just do not use its methods since it may change without any deprecation cycle
+            result.add((ExtendedCell) createCell(row, family, qualifier, timestamp, sum));
           }
           return moreRows;
         }
@@ -114,7 +118,9 @@ public boolean next(List<Cell> result, ScannerContext scannerContext) throws IOE
           row = CellUtil.cloneRow(firstCell);
           qualifier = CellUtil.cloneQualifier(firstCell);
         } else if (!CellUtil.matchingRows(firstCell, row)) {
-          result.add(createCell(row, family, qualifier, timestamp, sum));
+          // all cells in HBase are ExtendedCells, so you are fine to cast it to ExtendedCell,
+          // just do not use its methods since it may change without any deprecation cycle
+          result.add((ExtendedCell) createCell(row, family, qualifier, timestamp, sum));
           row = CellUtil.cloneRow(firstCell);
           qualifier = CellUtil.cloneQualifier(firstCell);
           sum = 0;
@@ -123,14 +129,18 @@ public boolean next(List<Cell> result, ScannerContext scannerContext) throws IOE
           if (CellUtil.matchingQualifier(c, qualifier)) {
             sum += Bytes.toLong(c.getValueArray(), c.getValueOffset());
           } else {
-            result.add(createCell(row, family, qualifier, timestamp, sum));
+            // all cells in HBase are ExtendedCells, so you are fine to cast it to ExtendedCell,
+            // just do not use its methods since it may change without any deprecation cycle
+            result.add((ExtendedCell) createCell(row, family, qualifier, timestamp, sum));
             qualifier = CellUtil.cloneQualifier(c);
             sum = Bytes.toLong(c.getValueArray(), c.getValueOffset());
           }
           timestamp = c.getTimestamp();
         });
         if (!moreRows) {
-          result.add(createCell(row, family, qualifier, timestamp, sum));
+          // all cells in HBase are ExtendedCells, so you are fine to cast it to ExtendedCell,
+          // just do not use its methods since it may change without any deprecation cycle
+          result.add((ExtendedCell) createCell(row, family, qualifier, timestamp, sum));
         }
         srcResult.clear();
         return moreRows;

hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapreduce/TestImportExport.java

@@ -830,7 +830,7 @@ private void checkWhetherTagExists(TableName table, boolean tagExists) throws IO
       // Need to use RegionScanner instead of table#getScanner since the latter will
       // not return tags since it will go through rpc layer and remove tags intentionally.
       RegionScanner scanner = region.getScanner(scan);
-      scanner.next((List) values);
+      scanner.next(values);
       if (!values.isEmpty()) {
         break;
       }

hbase-server/src/main/java/org/apache/hadoop/hbase/mob/DefaultMobStoreCompactor.java

@@ -362,7 +362,7 @@ protected boolean performCompaction(FileDetails fd, InternalScanner scanner, Cel
       fileName = Bytes.toBytes(mobFileWriter.getPath().getName());
 
       do {
-        hasMore = scanner.next((List) cells, scannerContext);
+        hasMore = scanner.next(cells, scannerContext);
         currentTime = EnvironmentEdgeManager.currentTime();
         if (LOG.isDebugEnabled()) {
           now = currentTime;

hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java

@@ -3297,7 +3297,7 @@ private void updateDeleteLatestVersionTimestamp(Cell cell, Get get, int count, b
     try (RegionScanner scanner = getScanner(new Scan(get))) {
       // NOTE: Please don't use HRegion.get() instead,
       // because it will copy cells to heap. See HBASE-26036
-      List<Cell> result = new ArrayList<>();
+      List<ExtendedCell> result = new ArrayList<>();
       scanner.next(result);
 
       if (result.size() < count) {
@@ -4070,7 +4070,7 @@ private Map<byte[], List<ExtendedCell>> reckonDeltas(Mutation mutation,
       for (Map.Entry<byte[], List<ExtendedCell>> entry : ClientInternalHelper
         .getExtendedFamilyCellMap(mutation).entrySet()) {
         final byte[] columnFamilyName = entry.getKey();
-        List<ExtendedCell> deltas = (List) entry.getValue();
+        List<ExtendedCell> deltas = entry.getValue();
         // Reckon for the Store what to apply to WAL and MemStore.
         List<ExtendedCell> toApply =
           reckonDeltasByStore(region.stores.get(columnFamilyName), mutation, now, deltas, results);
@@ -4131,7 +4131,7 @@ private List<ExtendedCell> reckonDeltasByStore(HStore store, Mutation mutation,
         // NOTE: Please don't use HRegion.get() instead,
         // because it will copy cells to heap. See HBASE-26036
         List<ExtendedCell> currentValues = new ArrayList<>();
-        scanner.next((List) currentValues);
+        scanner.next(currentValues);
         // Iterate the input columns and update existing values if they were found, otherwise
         // add new column initialized to the delta amount
         int currentValuesIndex = 0;
@@ -5063,7 +5063,7 @@ private CheckAndMutateResult checkAndMutateInternal(CheckAndMutate checkAndMutat
         try (RegionScanner scanner = getScanner(new Scan(get))) {
           // NOTE: Please don't use HRegion.get() instead,
           // because it will copy cells to heap. See HBASE-26036
-          List<Cell> result = new ArrayList<>(1);
+          List<ExtendedCell> result = new ArrayList<>(1);
           scanner.next(result);
           if (filter != null) {
             if (!result.isEmpty()) {
@@ -5079,7 +5079,7 @@ private CheckAndMutateResult checkAndMutateInternal(CheckAndMutate checkAndMutat
               matches = (result.get(0).getValueLength() == 0) == (op != CompareOperator.NOT_EQUAL);
               cellTs = result.get(0).getTimestamp();
             } else if (result.size() == 1) {
-              Cell kv = result.get(0);
+              ExtendedCell kv = result.get(0);
               cellTs = kv.getTimestamp();
               int compareResult = PrivateCellUtil.compareValue(kv, comparator);
               matches = matches(op, compareResult);

hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/InternalScanner.java

@@ -20,7 +20,7 @@
 import java.io.Closeable;
 import java.io.IOException;
 import java.util.List;
-import org.apache.hadoop.hbase.Cell;
+import org.apache.hadoop.hbase.ExtendedCell;
 import org.apache.hadoop.hbase.HBaseInterfaceAudience;
 import org.apache.yetus.audience.InterfaceAudience;
 import org.apache.yetus.audience.InterfaceStability;
@@ -41,21 +41,37 @@
 public interface InternalScanner extends Closeable {
   /**
    * Grab the next row's worth of values.
-   * @param result return output array
+   * <p>
+   * The generic type for the output list {@code result} means we will only add {@link ExtendedCell}
+   * to it. This is useful for the code in HBase as we can pass List&lt;ExtendedCell&gt; here to
+   * avoid casting, but may cause some troubles for coprocessors which implement this method. In
+   * general, all cells created via the {@link org.apache.hadoop.hbase.CellBuilder} are actually
+   * {@link ExtendedCell}s, so if you want to add something to the {@code result} list, you can just
+   * cast it to {@link ExtendedCell}, although it is marked as IA.Private.
+   * @param result return output array. We will only add ExtendedCell to this list, but for CP
+   *               users, you'd better just use {@link org.apache.hadoop.hbase.RawCell} as
+   *               {@link ExtendedCell} is IA.Private.
    * @return true if more rows exist after this one, false if scanner is done
-   * @throws IOException e
    */
-  default boolean next(List<Cell> result) throws IOException {
+  default boolean next(List<? super ExtendedCell> result) throws IOException {
     return next(result, NoLimitScannerContext.getInstance());
   }
 
   /**
    * Grab the next row's worth of values.
-   * @param result return output array
+   * <p>
+   * The generic type for the output list {@code result} means we will only add {@link ExtendedCell}
+   * to it. This is useful for the code in HBase as we can pass List&lt;ExtendedCell&gt; here to
+   * avoid casting, but may cause some troubles for coprocessors which implement this method. In
+   * general, all cells created via the {@link org.apache.hadoop.hbase.CellBuilder} are actually
+   * {@link ExtendedCell}s, so if you want to add something to the {@code result} list, you can just
+   * cast it to {@link ExtendedCell}, although it is marked as IA.Private.
+   * @param result return output array. We will only add ExtendedCell to this list, but for CP
+   *               users, you'd better just use {@link org.apache.hadoop.hbase.RawCell} as
+   *               {@link ExtendedCell} is IA.Private.
    * @return true if more rows exist after this one, false if scanner is done
-   * @throws IOException e
    */
-  boolean next(List<Cell> result, ScannerContext scannerContext) throws IOException;
+  boolean next(List<? super ExtendedCell> result, ScannerContext scannerContext) throws IOException;
 
   /**
    * Closes the scanner and releases any resources it has allocated

hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/KeyValueHeap.java

@@ -144,7 +144,8 @@ public ExtendedCell next() throws IOException {
    * @return true if more rows exist after this one, false if scanner is done
    */
   @Override
-  public boolean next(List<Cell> result, ScannerContext scannerContext) throws IOException {
+  public boolean next(List<? super ExtendedCell> result, ScannerContext scannerContext)
+    throws IOException {
     if (this.current == null) {
       return scannerContext.setScannerState(NextState.NO_MORE_VALUES).hasMoreValues();
     }

hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreCompactorSegmentsIterator.java

@@ -148,7 +148,7 @@ private boolean refillKVS() {
         // InternalScanner is for CPs so we do not want to leak ExtendedCell to the interface, but
         // all the server side implementation should only add ExtendedCell to the List, otherwise it
         // will cause serious assertions in our code
-        hasMore = compactingScanner.next((List) kvs, scannerContext);
+        hasMore = compactingScanner.next(kvs, scannerContext);
       } catch (IOException e) {
         // should not happen as all data are in memory
         throw new IllegalStateException(e);

hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MobStoreScanner.java

@@ -21,7 +21,6 @@
 import java.util.ArrayList;
 import java.util.List;
 import java.util.NavigableSet;
-import org.apache.hadoop.hbase.Cell;
 import org.apache.hadoop.hbase.ExtendedCell;
 import org.apache.hadoop.hbase.client.Scan;
 import org.apache.hadoop.hbase.mob.MobCell;
@@ -64,7 +63,7 @@ public MobStoreScanner(HStore store, ScanInfo scanInfo, Scan scan,
    * the mob file as the result.
    */
   @Override
-  public boolean next(List<Cell> outResult, ScannerContext ctx) throws IOException {
+  public boolean next(List<? super ExtendedCell> outResult, ScannerContext ctx) throws IOException {
     boolean result = super.next(outResult, ctx);
     if (!rawMobScan) {
       // retrieve the mob data

hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionScanner.java

@@ -19,7 +19,7 @@
 
 import java.io.IOException;
 import java.util.List;
-import org.apache.hadoop.hbase.Cell;
+import org.apache.hadoop.hbase.ExtendedCell;
 import org.apache.hadoop.hbase.HBaseInterfaceAudience;
 import org.apache.hadoop.hbase.client.RegionInfo;
 import org.apache.yetus.audience.InterfaceAudience;
@@ -78,7 +78,7 @@ default String getOperationId() {
    * @return true if more rows exist after this one, false if scanner is done
    * @throws IOException e
    */
-  boolean nextRaw(List<Cell> result) throws IOException;
+  boolean nextRaw(List<? super ExtendedCell> result) throws IOException;
 
   /**
    * Grab the next row's worth of values. The {@link ScannerContext} is used to enforce and track
@@ -109,5 +109,6 @@ default String getOperationId() {
    * @return true if more rows exist after this one, false if scanner is done
    * @throws IOException e
    */
-  boolean nextRaw(List<Cell> result, ScannerContext scannerContext) throws IOException;
+  boolean nextRaw(List<? super ExtendedCell> result, ScannerContext scannerContext)
+    throws IOException;
 }