[GitHub] [hbase] saintstack commented on a change in pull request #2498: HBASE-24996 Support CheckAndMutate in Region.batchMutate()

[GitBox <gi...@apache.org>]

saintstack commented on a change in pull request #2498:
URL: https://github.com/apache/hbase/pull/2498#discussion_r502653844



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HMobStore.java
##########
@@ -139,7 +139,8 @@ public Configuration getConfiguration() {
    */
   @Override
   protected KeyValueScanner createScanner(Scan scan, ScanInfo scanInfo,
-      NavigableSet<byte[]> targetCols, long readPt) throws IOException {
+    NavigableSet<byte[]> targetCols, long readPt,List<KeyValueScanner> additionalScanners)

Review comment:
       Has to be a KVScanner? Can't be a CellScanner? (Took a look... seems like the latter is something else...)

##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
##########
@@ -3968,19 +4069,27 @@ private Result doCoprocessorPreCallAfterRowLock(Mutation mutation) throws IOExce
      *   values. Side effect is our filling out of the <code>results</code> List.
      */
     private List<Cell> reckonDeltasByStore(HStore store, Mutation mutation, long now,
-      List<Cell> deltas, List<Cell> results) throws IOException {
+      List<Cell> deltas, List<Cell> results,
+      Map<byte[], SortedSet<Cell>> additionalCellsForStores) throws IOException {
       assert mutation instanceof Increment || mutation instanceof Append;
       byte[] columnFamily = store.getColumnFamilyDescriptor().getName();
       List<Pair<Cell, Cell>> cellPairs = new ArrayList<>(deltas.size());
 
       // Get previous values for all columns in this family.
+      Get get = new Get(mutation.getRow());
+      for (Cell cell: deltas) {
+        get.addColumn(columnFamily, CellUtil.cloneQualifier(cell));
+      }
       TimeRange tr;
       if (mutation instanceof Increment) {
         tr = ((Increment) mutation).getTimeRange();
       } else {
         tr = ((Append) mutation).getTimeRange();
+      }      if (tr != null) {

Review comment:
       Formatting

##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/Region.java
##########
@@ -291,16 +291,13 @@
 
   /**
    * Perform a batch of mutations.
-   * <p>
-   * Note this supports only Put, Delete, Increment and Append mutations and will ignore other
-   * types passed.
+   *

Review comment:
       Nice. One way in only.

##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
##########
@@ -3069,27 +3073,33 @@ private RegionScannerImpl getScanner(Scan scan, List<KeyValueScanner> additional
           checkFamily(family);
         }
       }
-      return instantiateRegionScanner(scan, additionalScanners, nonceGroup, nonce);
+      return instantiateRegionScanner(scan, additionalScanners, additionalScannersForStores,

Review comment:
       Whats the diff between the two additional Scanner?

##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
##########
@@ -4074,29 +4183,128 @@ private static long getLongValue(final Cell cell) throws DoNotRetryIOException {
       return PrivateCellUtil.getValueAsLong(cell);
     }
 
-    /**
-     * Do a specific Get on passed <code>columnFamily</code> and column qualifiers.
-     * @param mutation Mutation we are doing this Get for.
-     * @param store Which column family on row (TODO: Go all Gets in one go)
-     * @param coordinates Cells from <code>mutation</code> used as coordinates applied to Get.
-     * @return Return list of Cells found.
-     */
-    private List<Cell> get(Mutation mutation, HStore store, List<Cell> coordinates,
-      TimeRange tr) throws IOException {
-      // Sort the cells so that they match the order that they appear in the Get results.
-      // Otherwise, we won't be able to find the existing values if the cells are not specified
-      // in order by the client since cells are in an array list.
-      // TODO: I don't get why we are sorting. St.Ack 20150107

Review comment:
       Anything on this sort issue?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org