kaisun2000 commented on a change in pull request #1441:
URL: https://github.com/apache/helix/pull/1441#discussion_r500609429
##########
File path: helix-core/src/main/java/org/apache/helix/GroupCommit.java
##########
@@ -139,14 +142,22 @@ public boolean commit(BaseDataAccessor<ZNRecord>
accessor, int options, String k
success = false;
while (++retry <= MAX_RETRY && !success) {
if (removeIfEmpty && merged.getMapFields().isEmpty()) {
- success = accessor.remove(mergedKey, options);
+ try {
+ success = accessor.remove(mergedKey, options);
+ } catch (Exception e) {
+ success = false;
Review comment:
The thing is that we did not log out the exception. At the time of
investigation of production issue, you always want to know what is the
exception thrown.
Current code actually "eat" the exception from IO (zk access). This is
normally not a good idea.
how about something like this?
```
try {
success = accessor.remove(mergedKey, options);
} catch (Exception e) {
LOG.error (" failed to remove {} from zk, retry it",
mergedkey, e);
return false;
}
LOG.info( "removed {}", mergedkey);
```
----------------------------------------------------------------
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]
