abstractdog commented on code in PR #427:
URL: https://github.com/apache/tez/pull/427#discussion_r2549661395


##########
tez-api/src/main/java/org/apache/tez/client/registry/AMRecord.java:
##########
@@ -150,14 +180,21 @@ public boolean equals(Object other) {
   public ServiceRecord toServiceRecord() {
     ServiceRecord serviceRecord = new ServiceRecord();
     serviceRecord.set(APP_ID_RECORD_KEY, appId);
-    serviceRecord.set(HOST_RECORD_KEY, host);
+    serviceRecord.set(HOST_NAME_RECORD_KEY, hostName);
+    serviceRecord.set(HOST_IP_RECORD_KEY, hostIp);
     serviceRecord.set(PORT_RECORD_KEY, port);
-    serviceRecord.set(OPAQUE_ID_KEY, id);
+    serviceRecord.set(EXTERNAL_ID_KEY, externalId);
+    serviceRecord.set(COMPUTE_GROUP_NAME_KEY, computeName);
     return serviceRecord;
   }
 
+  @Override
+  public String toString() {
+    return toServiceRecord().attributes().toString();
+  }

Review Comment:
   used in INFO level messages in downstream-only tez session client 
implementation, and toServiceRecord itself is used in ZkAMRegistry to store AM 
information in zookeeper as JSON
   
   would you be fine to cache ServiceRecord in a field and return it like:
   ```
     public ServiceRecord toServiceRecord() {
       if (serviceRecord != null) {
         return serviceRecord;
       }
       serviceRecord = new ServiceRecord();
       serviceRecord.set(APP_ID_RECORD_KEY, appId);
       serviceRecord.set(HOST_NAME_RECORD_KEY, hostName);
       serviceRecord.set(HOST_IP_RECORD_KEY, hostIp);
       serviceRecord.set(PORT_RECORD_KEY, port);
       serviceRecord.set(EXTERNAL_ID_KEY, externalId);
       serviceRecord.set(COMPUTE_GROUP_NAME_KEY, computeName);
   
       return serviceRecord;
     }
   ```



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

Reply via email to