lcspinter commented on a change in pull request #2507:
URL: https://github.com/apache/hive/pull/2507#discussion_r673815217



##########
File path: 
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnStore.java
##########
@@ -388,8 +388,17 @@ void onRename(String oldCatName, String oldDbName, String 
oldTabName, String old
    * @param workerVersion runtime version of the worker calling this
    * @return an info element for this compaction request, or null if there is 
no work to do now.
    */
+  @Deprecated

Review comment:
       Again, comment about what to use instead of this deprecated method.

##########
File path: 
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
##########
@@ -165,16 +166,29 @@ public CompactionTxnHandler() {
     }
   }
 
+
   /**
    * This will grab the next compaction request off of
    * the queue, and assign it to the worker.
    * @param workerId id of the worker calling this, will be recorded in the db
-   * @param workerVersion runtime version of the Worker calling this
    * @return an info element for this compaction request, or null if there is 
no work to do now.
    */
+  @Deprecated

Review comment:
       Could you please add some comments, what to use instead of this method? 

##########
File path: 
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/CompactionTxnHandler.java
##########
@@ -204,7 +218,8 @@ public CompactionInfo findNextToCompact(String workerId, 
String workerVersion) t
           info.properties = rs.getString(6);
           // Now, update this record as being worked on by this worker.
           long now = getDbTime(dbConn);
-          s = "UPDATE \"COMPACTION_QUEUE\" SET \"CQ_WORKER_ID\" = '" + 
workerId + "', \"CQ_WORKER_VERSION\" = '" + workerVersion + "', " +
+          s = "UPDATE \"COMPACTION_QUEUE\" SET \"CQ_WORKER_ID\" = '" + 
rqst.getWorkerId() + "', " +
+            "\"CQ_WORKER_VERSION\" = '" + rqst.getWorkerVersion() + "', " +

Review comment:
       I believe the `getWorkerVersion()` can be null, if this method is called 
from `findNextToCompact(String workerId)`. Is `CQ_WORKER_VERSION='null'` is a 
valid scenario? Are we prepared to handle null values when processing the 
`CQ_WORKER_VERSION` column? 




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

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to