Skip to content

Commit

Permalink
respond reviewer feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
GrantPSpencer committed Jan 24, 2025
1 parent 98926d0 commit 696a245
Showing 1 changed file with 20 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import java.util.Arrays;
import java.util.Collections;
import java.util.Date;
import java.util.HashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -1796,9 +1797,7 @@ public boolean deleteRecursive(String path) {
}

/**
* Delete the path as well as all its children. This operation is not atomic and may result in a partial deletion.
* This operation will handle concurrent deletions to the tree by another agent, but will not be able to handle
* concurrent creations.
* Delete the path as well as all its children.
* @param path
* @throws ZkClientException
*/
Expand Down Expand Up @@ -1846,25 +1845,34 @@ public void deleteRecursivelyAtomic(List<String> paths) {
for (String path : paths) {
ops.addAll(getOpsForRecursiveDelete(path));
}

// Return early if no operations to execute
if (ops.isEmpty()) {
return;
}

try {
opResults = multi(ops);
} catch (Exception e) {
LOG.error("zkclient {}, Failed to delete paths {}, exception {}", _uid, paths, e);
throw new ZkClientException("Failed to delete paths " + paths, e);
}

List<KeeperException.Code> opResultErrorCodes = new ArrayList<>();
for (OpResult result : opResults) {
if (result instanceof OpResult.ErrorResult) {
opResultErrorCodes.add(KeeperException.Code.get(((OpResult.ErrorResult) result).getErr()));
// Check if any of the operations failed. Create mapping of failed paths to error codes
Map<String, KeeperException.Code> failedPathsMap = new HashMap<>();
for (int i = 0; i < opResults.size(); i++) {
if (opResults.get(i) instanceof OpResult.ErrorResult) {
failedPathsMap.put(ops.get(i).getPath(),
KeeperException.Code.get(((OpResult.ErrorResult) opResults.get(i)).getErr()));
}
}

if (!opResultErrorCodes.isEmpty()) {
LOG.error("zkclient {}, Failed to delete paths {}, multi returned with error codes {}",
_uid, paths, opResultErrorCodes);
// Log and throw exception if any of the operations failed
if (!failedPathsMap.isEmpty()) {
LOG.error("zkclient {}, Failed to delete paths {}, multi returned with error codes {} for sub-paths {}",
_uid, paths, failedPathsMap.keySet(), failedPathsMap.values());
throw new ZkClientException("Failed to delete paths " + paths + " with ZK KeeperException error codes: "
+ opResultErrorCodes);
+ failedPathsMap.keySet() + " for paths: " + failedPathsMap.values());
}
}

Expand All @@ -1876,7 +1884,7 @@ public void deleteRecursivelyAtomic(List<String> paths) {
*/
private List<Op> getOpsForRecursiveDelete(String root) {
List<Op> ops = new ArrayList<>();
// Return early if the root does not exist
// Return empty list if the root does not exist
if (!exists(root)) {
return ops;
}
Expand Down

0 comments on commit 696a245

Please sign in to comment.