[GitHub] [hbase] brfrn169 commented on a change in pull request #2953: HBASE-25574 Revisit increment/append related RegionObserver methods

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

brfrn169 commented on a change in pull request #2953:
URL: https://github.com/apache/hbase/pull/2953#discussion_r578092094



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/RegionObserver.java
##########
@@ -968,12 +968,34 @@ default CheckAndMutateResult postCheckAndMutate(ObserverContext<RegionCoprocesso
    * @param c the environment provided by the region server
    * @param append Append object
    * @return result to return to the client if bypassing default processing
+   * @deprecated since 3.0.0 and will be removed in 4.0.0. Use
+   *   {@link #preAppend(ObserverContext, Append, WALEdit, Durability)} instead.
    */
+  @Deprecated
   default Result preAppend(ObserverContext<RegionCoprocessorEnvironment> c, Append append)
-      throws IOException {
+    throws IOException {
     return null;
   }
 
+  /**
+   * Called before Append.
+   * <p>
+   * Call CoprocessorEnvironment#bypass to skip default actions.
+   * If 'bypass' is set, we skip out on calling any subsequent chained coprocessors.
+   * <p>
+   * Note: Do not retain references to any Cells in 'append' beyond the life of this invocation.
+   * If need a Cell reference for later use, copy the cell and use that.
+   * @param c the environment provided by the region server
+   * @param append Append object
+   * @param edit The WALEdit object that will be written to the wal
+   * @param durability Persistence guarantee for this Put

Review comment:
       Thank you for taking a look at this!
   
   > Persistence guarantee for this 'Put'? Should be Append?
   
   Yes, this is a typo. I will fix it.
   
   > And could it be different from the durability of the given append object?  Checking the later code, it seems that we just pass in the m.getDurability, so do we really need this parameter?
   
   It is the same as the given append object. I added the `durability` parameter because the `prePut`/`preDelete`/`postPut`/`postDelete` methods have this parameter. I don't think we really need this parameter. Should we remove it from `preAppend`/`preIncrement`/`postAppend`/`postIncrement`? Or keep it for the consistency perspective?




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