zhangmeng916 commented on a change in pull request #1441:
URL: https://github.com/apache/helix/pull/1441#discussion_r500605834
##########
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:
if you look at the next line. It's already logged there.
##########
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;
+ }
if (!success) {
LOG.error("Fails to remove " + mergedKey + " from ZK, retry
it!");
} else {
LOG.info("Removed " + mergedKey);
}
} else {
- success = accessor.set(mergedKey, merged, options);
+ try {
+ success = accessor.set(mergedKey, merged, options);
+ } catch (Exception e) {
+ success = false;
Review comment:
It's logged next line.
##########
File path:
helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java
##########
@@ -1315,8 +1315,11 @@ public void addTypeToCustomizedStateConfig(String
clusterName, String type) {
ZKHelixDataAccessor accessor =
new ZKHelixDataAccessor(clusterName, new
ZkBaseDataAccessor<ZNRecord>(_zkClient));
PropertyKey.Builder keyBuilder = accessor.keyBuilder();
- accessor.updateProperty(keyBuilder.customizedStateConfig(),
- customizedStateConfigFromBuilder);
+ if(!accessor.updateProperty(keyBuilder.customizedStateConfig(),
Review comment:
The reason we need this PR is that we need to make sure updateProperty
with group commit or non group commit have the same behavior. Currently non
group commit update return error code, while group commit throws exception.
This should not impact any callbackhandler existing logic.
----------------------------------------------------------------
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]
