Copilot commented on code in PR #7078:
URL: https://github.com/apache/hbase/pull/7078#discussion_r2311960107
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/procedure/ZKProcedureCoordinator.java:
##########
@@ -174,6 +174,72 @@ final public void resetMembers(Procedure proc) throws
IOException {
} while (stillGettingNotifications);
}
+ /**
+ * Delete acquired barrier node that are no longer in use.
+ */
+ @Override
+ final public void cleanupBarrierAcquired(Procedure proc) throws IOException {
+ String procName = proc.getName();
+ boolean stillGettingNotifications = false;
+ do {
+ try {
+ LOG.debug("Attempting to clean up acquired barrier node for
procedure:" + procName);
Review Comment:
Missing space after colon in log messages. Should be 'procedure: ' +
procName for consistency with typical log formatting.
```suggestion
LOG.debug("Attempting to clean up acquired barrier node for
procedure: " + procName);
```
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/procedure/Procedure.java:
##########
@@ -274,7 +275,9 @@ public void sendGlobalBarrierReached() throws
ForeignException {
public void sendGlobalBarrierComplete() {
LOG.debug("Finished coordinator procedure - removing self from list of
running procedures");
try {
- coord.getRpcs().resetMembers(this);
+ coord.getRpcs().cleanupBarrierAcquired(this);
Review Comment:
The cleanupBarrierAcquired method is called in both
sendGlobalBarrierReached() and sendGlobalBarrierComplete(). This duplication
could lead to unnecessary cleanup operations and should be reviewed to ensure
it's intentional.
```suggestion
```
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/procedure/Procedure.java:
##########
@@ -259,6 +259,7 @@ public void sendGlobalBarrierStart() throws
ForeignException {
*/
public void sendGlobalBarrierReached() throws ForeignException {
try {
+ coord.getRpcs().cleanupBarrierAcquired(this);
Review Comment:
The cleanupBarrierAcquired method is called in both
sendGlobalBarrierReached() and sendGlobalBarrierComplete(). This duplication
could lead to unnecessary cleanup operations and should be reviewed to ensure
it's intentional.
```suggestion
```
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/procedure/ZKProcedureCoordinator.java:
##########
@@ -174,6 +174,72 @@ final public void resetMembers(Procedure proc) throws
IOException {
} while (stillGettingNotifications);
}
+ /**
+ * Delete acquired barrier node that are no longer in use.
+ */
+ @Override
+ final public void cleanupBarrierAcquired(Procedure proc) throws IOException {
+ String procName = proc.getName();
+ boolean stillGettingNotifications = false;
+ do {
+ try {
+ LOG.debug("Attempting to clean up acquired barrier node for
procedure:" + procName);
+ zkProc.clearAcquiredBarrierNode(procName);
+ stillGettingNotifications = false;
+ } catch (KeeperException.NotEmptyException e) {
+ stillGettingNotifications = true;
+ } catch (KeeperException e) {
+ String msg = "Failed to complete clean up acquired barrier node for
procedure " + procName;
+ LOG.error(msg, e);
+ throw new IOException(msg, e);
+ }
+ } while (stillGettingNotifications);
+ }
+
+ /**
+ * Delete reached barrier node that are no longer in use.
+ */
+ @Override
+ final public void cleanupBarrierReached(Procedure proc) throws IOException {
+ String procName = proc.getName();
+ boolean stillGettingNotifications = false;
+ do {
+ try {
+ LOG.debug("Attempting to clean up reached barrier node for procedure:"
+ procName);
+ zkProc.clearReachedBarrierNode(procName);
+ stillGettingNotifications = false;
+ } catch (KeeperException.NotEmptyException e) {
+ stillGettingNotifications = true;
+ } catch (KeeperException e) {
+ String msg = "Failed to complete clean up reached barrier node for
procedure " + procName;
+ LOG.error(msg, e);
+ throw new IOException(msg, e);
+ }
+ } while (stillGettingNotifications);
+ }
+
+ /**
+ * Delete aborted node that are no longer in use.
+ */
+ @Override
+ final public void cleanupBarrierAborted(Procedure proc) throws IOException {
+ String procName = proc.getName();
+ boolean stillGettingNotifications = false;
+ do {
+ try {
+ LOG.debug("Attempting to clean up aborted node for procedure:" +
procName);
Review Comment:
Missing space after colon in log messages. Should be 'procedure: ' +
procName for consistency with typical log formatting.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/procedure/ZKProcedureCoordinator.java:
##########
@@ -174,6 +174,72 @@ final public void resetMembers(Procedure proc) throws
IOException {
} while (stillGettingNotifications);
}
+ /**
+ * Delete acquired barrier node that are no longer in use.
+ */
+ @Override
+ final public void cleanupBarrierAcquired(Procedure proc) throws IOException {
+ String procName = proc.getName();
+ boolean stillGettingNotifications = false;
+ do {
+ try {
+ LOG.debug("Attempting to clean up acquired barrier node for
procedure:" + procName);
+ zkProc.clearAcquiredBarrierNode(procName);
+ stillGettingNotifications = false;
+ } catch (KeeperException.NotEmptyException e) {
+ stillGettingNotifications = true;
+ } catch (KeeperException e) {
+ String msg = "Failed to complete clean up acquired barrier node for
procedure " + procName;
+ LOG.error(msg, e);
+ throw new IOException(msg, e);
+ }
+ } while (stillGettingNotifications);
+ }
+
+ /**
+ * Delete reached barrier node that are no longer in use.
+ */
+ @Override
+ final public void cleanupBarrierReached(Procedure proc) throws IOException {
+ String procName = proc.getName();
+ boolean stillGettingNotifications = false;
+ do {
+ try {
+ LOG.debug("Attempting to clean up reached barrier node for procedure:"
+ procName);
Review Comment:
Missing space after colon in log messages. Should be 'procedure: ' +
procName for consistency with typical log formatting.
```suggestion
LOG.debug("Attempting to clean up reached barrier node for
procedure: " + procName);
```
--
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]
