smengcl commented on code in PR #10761:
URL: https://github.com/apache/ozone/pull/10761#discussion_r4227072620
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/local/LocalOzoneCluster.java:
##########
@@ -372,6 +423,15 @@ private void stopS3Gateway() throws Exception {
}
}
+ private void stopRecon() {
+ ReconServer service = reconServer;
+ reconServer = null;
+ if (service != null) {
+ service.stop();
+ service.join();
Review Comment:
This stop/join pair leaves Recon's SCM sync scheduler running. I reproduced
this with a real S3+Recon
cluster: close() returned and main() finished, but the non-daemon
Recon-SyncSCMContainerInfo-0 thread
kept the JVM alive.
ReconStorageContainerManagerFacade.start() creates the scheduler as a local
variable, so stop()
cannot shut it down. Please retain it as a field and stop it before closing
the other subsystems:
```diff
private final ExecutorService scmSnapshotExecutor;
+ private ScheduledExecutorService scmContainerSyncScheduler;
@@ start()
- ScheduledExecutorService scheduler = Executors.newScheduledThreadPool(1,
+ scmContainerSyncScheduler = Executors.newScheduledThreadPool(1,
- scheduler.scheduleWithFixedDelay(() -> {
+ scmContainerSyncScheduler.scheduleWithFixedDelay(() -> {
@@ stop()
+ if (scmContainerSyncScheduler != null) {
+ scmContainerSyncScheduler.shutdownNow();
+ }
getDatanodeProtocolServer().stop();
```
Please also add a regression test for scheduler shutdown or natural JVM exit.
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/local/LocalOzoneCluster.java:
##########
@@ -332,6 +366,23 @@ InetSocketAddress getS3gBoundAddress() {
return s3Gateway != null ? s3Gateway.getHttpAddress() : null;
}
+ @Override
+ public int getReconPort() {
+ if (!config.isReconEnabled()) {
+ return -1;
+ }
+ return preparedConfiguration == null ? config.getReconPort()
+ : preparedConfiguration.getReconPort();
+ }
+
+ @Override
+ public String getReconEndpoint() {
+ if (!config.isReconEnabled()) {
+ return "";
+ }
+ return "http://"; + getDisplayHost() + ":" + getReconPort();
Review Comment:
With --no-s3g --recon and ozone.http.policy=HTTPS_ONLY, preparation
succeeds, but this returns an HTTP
endpoint. Recon disables its HTTP listener under that policy, so startup
probes an endpoint that
cannot respond and eventually times out.
Please move the existing HTTP-policy validation from configureS3Gateway()
into
configureLocalDefaults(), applying it when either S3 Gateway or Recon is
enabled:
```diff
private void configureLocalDefaults(OzoneConfiguration conf) throws
IOException {
+ if (config.isS3gEnabled() || config.isReconEnabled()) {
+ HttpConfig.Policy httpPolicy =
+ HttpConfig.Policy.fromString(conf.get(OZONE_HTTP_POLICY_KEY,
OZONE_HTTP_POLICY_DEFAULT));
+ if (httpPolicy != null && !httpPolicy.isHttpEnabled()) {
+ rejectUserConfigured(conf, OZONE_HTTP_POLICY_KEY, "HTTP_ONLY or
HTTP_AND_HTTPS");
+ }
+ }
```
Remove the existing copy from configureS3Gateway() and add a
--no-s3g/Recon/HTTPS_ONLY rejection test.
--
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]
