dasahcc commented on a change in pull request #1523:
URL: https://github.com/apache/helix/pull/1523#discussion_r520808899



##########
File path: 
helix-core/src/main/java/org/apache/helix/common/caches/TaskDataCache.java
##########
@@ -174,6 +178,13 @@ private void refreshContexts(HelixDataAccessor accessor) {
       }
     }
 
+    for (Map.Entry<String, ZNRecord> entry : _contextMap.entrySet()) {
+      if (entry.getValue() != null) {
+        _initialContextMapHashcode.put(entry.getKey(),
+            Objects.hashCode(entry.getValue().toString()));

Review comment:
       Do we need to use Objects.hashCode? Since we already converted it to 
String, we can call the String's function hashCode();

##########
File path: 
helix-core/src/main/java/org/apache/helix/common/caches/TaskDataCache.java
##########
@@ -267,6 +278,11 @@ public void updateWorkflowContext(String resourceName, 
WorkflowContext workflowC
    * Update context of the Workflow or Job
    */
   private void updateContext(String resourceName, ZNRecord record) {

Review comment:
       I think this may be an overkill by checking every update. We can let it 
keep updating but just one check for all the context before doing writes to ZK. 
Otherwise, say I am trying to update one context 10 times, I need to convert to 
String every time. 
   
   Also we can leverage Java8 stream to do the checks before write to ZK, it 
could be efficient instead of every update check.




----------------------------------------------------------------
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:
[email protected]



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

Reply via email to