Re: [PR] HDDS-12527. Separate S3 Gateway from MiniOzoneCluster [ozone]
adoroszlai commented on code in PR #8058:
URL: https://github.com/apache/ozone/pull/8058#discussion_r1991364821
##
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/MiniOzoneClusterImpl.java:
##
@@ -531,16 +511,18 @@ private static void stopRecon(ReconServer reconServer) {
}
}
- private static void stopS3G(Gateway s3g) {
-try {
- if (s3g != null) {
-LOG.info("Stopping S3G");
-// TODO (HDDS-11539): Remove this workaround once the @PreDestroy
issue is fixed
-OzoneClientCache.closeClient();
-s3g.stop();
+ private static void stopServices(List services) {
+// stop in reverse order
+List reverse = new ArrayList<>(services);
+Collections.reverse(reverse);
+
+for (Service service : reverse) {
+ try {
+service.stop();
+LOG.info("Stopped {}", service);
+ } catch (Exception e) {
+LOG.error("Error stopping {}", service, e);
Review Comment:
Thanks for the suggestion. I'll keep the original, because I don't expect
many services (only Recon (HDDS-12568) and maybe S3G HA (HDDS-11679)).
This is just a precaution to allow dependent services to be added in
dependency order.
--
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]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HDDS-12527. Separate S3 Gateway from MiniOzoneCluster [ozone]
adoroszlai commented on PR #8058: URL: https://github.com/apache/ozone/pull/8058#issuecomment-2718090227 Thanks @ivandika3, @peterxcli for the review. -- 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] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HDDS-12527. Separate S3 Gateway from MiniOzoneCluster [ozone]
adoroszlai merged PR #8058: URL: https://github.com/apache/ozone/pull/8058 -- 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] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] HDDS-12527. Separate S3 Gateway from MiniOzoneCluster [ozone]
peterxcli commented on code in PR #8058:
URL: https://github.com/apache/ozone/pull/8058#discussion_r1991300952
##
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/MiniOzoneClusterImpl.java:
##
@@ -531,16 +511,18 @@ private static void stopRecon(ReconServer reconServer) {
}
}
- private static void stopS3G(Gateway s3g) {
-try {
- if (s3g != null) {
-LOG.info("Stopping S3G");
-// TODO (HDDS-11539): Remove this workaround once the @PreDestroy
issue is fixed
-OzoneClientCache.closeClient();
-s3g.stop();
+ private static void stopServices(List services) {
+// stop in reverse order
+List reverse = new ArrayList<>(services);
+Collections.reverse(reverse);
+
+for (Service service : reverse) {
+ try {
+service.stop();
+LOG.info("Stopped {}", service);
+ } catch (Exception e) {
+LOG.error("Error stopping {}", service, e);
Review Comment:
nit:
```suggestion
ListIterator iterator = services.listIterator(services.size());
while (iterator.hasPrevious()) {
Service service = iterator.previous();
try {
service.stop();
LOG.info("Stopped {}", service);
} catch (Exception e) {
LOG.error("Error stopping {}", service, e);
```
However, this has worse readability, so please take it with a grain of salt.
--
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]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HDDS-12527. Separate S3 Gateway from MiniOzoneCluster [ozone]
peterxcli commented on code in PR #8058:
URL: https://github.com/apache/ozone/pull/8058#discussion_r1991300952
##
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/MiniOzoneClusterImpl.java:
##
@@ -531,16 +511,18 @@ private static void stopRecon(ReconServer reconServer) {
}
}
- private static void stopS3G(Gateway s3g) {
-try {
- if (s3g != null) {
-LOG.info("Stopping S3G");
-// TODO (HDDS-11539): Remove this workaround once the @PreDestroy
issue is fixed
-OzoneClientCache.closeClient();
-s3g.stop();
+ private static void stopServices(List services) {
+// stop in reverse order
+List reverse = new ArrayList<>(services);
+Collections.reverse(reverse);
+
+for (Service service : reverse) {
+ try {
+service.stop();
+LOG.info("Stopped {}", service);
+ } catch (Exception e) {
+LOG.error("Error stopping {}", service, e);
Review Comment:
nit:
```suggestion
ListIterator iterator = services.listIterator(services.size());
while (iterator.hasPrevious()) {
Service service = iterator.previous();
try {
service.stop();
LOG.info("Stopped {}", service);
} catch (Exception e) {
LOG.error("Error stopping {}", service, e);
}
}
```
However, this has worse readability, so please take it with a grain of salt.
--
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]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] HDDS-12527. Separate S3 Gateway from MiniOzoneCluster [ozone]
adoroszlai commented on PR #8058: URL: https://github.com/apache/ozone/pull/8058#issuecomment-2717506899 @ivandika3 @peterxcli please take a look -- 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] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
