HDDS-8253. Set ozone.metadata.dirs to temporary dir if not defined in S3 Gateway #4455

swamirishi · 2023-03-23T04:33:32Z

What changes were proposed in this pull request?

Currently s3Gateway fails to come up if "ozone.http.basedir" is not defined.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-8253

How was this patch tested?

Manually tested.

…teway

kerneltime · 2023-03-23T05:33:00Z

I took a deeper look into this, this was due to the change in #4308 which impacts S3G as S3 G does configure a webui that displays some basic AWS CLI usage. One option could be to set the temporary folder to be relative to the current working directory for S3G. The main issue that is being avoided in #4308 is the cleanup of the landing page content due to clean up of /tmp folder.

hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/Gateway.java

adoroszlai · 2023-03-28T08:32:34Z

hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/Gateway.java

+      // fail.
+      Path tmpMetaDir = Files.createTempDirectory(Paths.get(""),
+              "ozone_s3g_tmp_meta_dir");
+      Runtime.getRuntime().addShutdownHook(new Thread(() -> {


Please use ShutdownHookManager to add the shutdown hook. Also, I think it should be executed after the other hook (which stops the server), so use a lower than default priority.

@swamirishi do you plan to address this? Also, we can change this PR to change the logic for all the webUIs to see the $CWD.

@kerneltime Should I do the web ui change as part of another PR?

hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/Gateway.java

adoroszlai

On second look I think it would be better to set OzoneConfigKeys.OZONE_HTTP_BASEDIR instead of OZONE_METADATA_DIRS, which is only a fallback.

ozone/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/server/http/BaseHttpServer.java

Lines 183 to 189 in 881a471

    
           String baseDir = conf.get(OzoneConfigKeys.OZONE_HTTP_BASEDIR); 
        
           if (StringUtils.isEmpty(baseDir)) { 
        
             baseDir = getOzoneMetaDirPath(conf) + SERVER_DIR; 
        
           } 
        
           createDir(baseDir); 
        
           httpServer.getWebAppContext().setAttribute(JETTY_BASETMPDIR, baseDir);

adoroszlai · 2023-03-30T06:36:46Z

Thanks @swamirishi for updating the patch to set OZONE_HTTP_BASEDIR. I think you missed the other two comments:

#4455 (comment)
#4455 (comment)

neils-dev · 2023-04-03T00:35:04Z

hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/Gateway.java

+    if (StringUtils.isEmpty(ozoneConfiguration.get(
+            OzoneConfigKeys.OZONE_HTTP_BASEDIR))) {
+      //Setting ozone.metadata.dirs if not set so that server setup doesn't
+      // fail.


Update comment, setting to tmp directory from current working directory, cwd.

neils-dev · 2023-04-11T04:58:17Z

Thanks @swamirishi . Following up on this, just have the minor comment regarding fixing the comment in the code to set to current working dir if not set. Also, the CI workflow has errors in the integration tests, re-run to see if resolves.

adoroszlai · 2023-04-11T05:42:26Z

CI workflow has errors in the integration tests, re-run to see if resolves

Since there are changes requested outstanding, CI does not need to be re-run with the current state of the PR.

adoroszlai · 2023-04-17T10:39:19Z

/pending extract method, use ShutdownHookManager

github-actions

Marking this issue as un-mergeable as requested.

Please use /ready comment when it's resolved.

Please note that the PR will be closed after 21 days of inactivity from now. (But can be re-opened anytime later...)

extract method, use ShutdownHookManager

adoroszlai

Thanks @swamirishi for updating the patch. I think it's getting close to completion, but there is a checkstyle failure.

hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/Gateway.java

adoroszlai · 2023-04-17T20:06:28Z

Thanks @swamirishi for the updates. Latest commit introduced another checkstyle failure:

hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/Gateway.java
 23: Unused import - java.nio.file.Path.

(Hint: it can be run locally simply by hadoop-ozone/dev-support/checks/checkstyle.sh.)

adoroszlai · 2023-04-18T07:25:11Z

/ready

Blocking review request is removed.

adoroszlai · 2023-04-18T10:29:51Z

Thanks @swamirishi for the patch, @kerneltime, @neils-dev, @sadanand48 for the review.

* master: (440 commits) HDDS-8445. Move PlacementPolicy back to SCM (apache#4588) HDDS-8335. ReplicationManager: EC Mis and Under replication handlers should handle overloaded exceptions (apache#4593) HDDS-8355. Intermittent failure in TestOMRatisSnapshots#testInstallSnapshot (apache#4592) HDDS-8444. Increase timeout of CI build (apache#4586) HDDS-8446. Selective checks: handle change in ci.yaml (apache#4587) HDDS-8440. Ozone Manager crashed with ClassCastException when deleting FSO bucket. (apache#4582) HDDS-7309. Enable by default GRPC between S3G and OM (apache#3820) HDDS-8458. Mark TestBlockDeletion#testBlockDeletion as flaky HDDS-8385. Ozone can't process snapshot when service UID > 2097151 (apache#4580) HDDS-8424: Preserve legacy bucket getKeyInfo behavior (apache#4576) HDDS-8453. Mark TestDirectoryDeletingServiceWithFSO#testDirDeletedTableCleanUpForSnapshot as flaky HDDS-8137. [Snapshot] SnapDiff to use tombstone entries in SST files (apache#4376) HDDS-8270. Measure checkAccess latency for Ozone objects (apache#4467) HDDS-8109. Seperate Ratis and EC MisReplication Handling (apache#4577) HDDS-8429. Checkpoint is not closed properly in OMDBCheckpointServlet (apache#4575) HDDS-8253. Set ozone.metadata.dirs to temporary dir if not defined in S3 Gateway (apache#4455) HDDS-8400. Expose rocksdb last sequence number through metrics (apache#4557) HDDS-8333. ReplicationManager: Allow partial EC reconstruction if insufficient nodes available (apache#4579) HDDS-8147. Introduce latency metrics for S3 Gateway operations (apache#4383) HDDS-7908. Support OM Metadata operation Generator in `Ozone freon` (apache#4251) ...

HDDS-8253: Set ozone.metadata.dirs if not defined to tmp dir for s3Ga…

e2a7d30

…teway

kerneltime reviewed Mar 23, 2023

View reviewed changes

hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/Gateway.java Outdated Show resolved Hide resolved

adoroszlai requested review from ChenSammi and sadanand48 March 23, 2023 08:53

sadanand48 reviewed Mar 23, 2023

View reviewed changes

hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/Gateway.java Outdated Show resolved Hide resolved

adoroszlai reviewed Mar 24, 2023

View reviewed changes

hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/Gateway.java Outdated Show resolved Hide resolved

HDDS-8253: Default Metadir in Current Working Directory

cea29f3

swamirishi force-pushed the HDDS-8253 branch from 1a94ee6 to cea29f3 Compare March 24, 2023 15:44

kerneltime requested a review from neils-dev March 27, 2023 16:24

adoroszlai requested changes Mar 28, 2023

View reviewed changes

adoroszlai reviewed Mar 28, 2023

View reviewed changes

swamirishi added 2 commits March 29, 2023 16:26

HDDS-8253: Set Basedir instead of Metadata dir

99273ee

HDDS-8253: Set Basedir instead of Metadata dir

669e89b

neils-dev reviewed Apr 3, 2023

View reviewed changes

github-actions bot previously requested changes Apr 17, 2023

View reviewed changes

github-actions bot added the pending label Apr 17, 2023

HDDS-8253. Use shutdown hook manager

d9da3b1

adoroszlai reviewed Apr 17, 2023

View reviewed changes

hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/Gateway.java Outdated Show resolved Hide resolved

hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/Gateway.java Outdated Show resolved Hide resolved

swamirishi added 4 commits April 17, 2023 11:07

HDDS-8253. Fix checkstyle issue

eda47dc

HDDS-8253. Fix checkstyle issue

7429fcd

HDDS-8253. Address review comments to move tmpMetaDatadir

df38baf

HDDS-8253. Fix comments

d992797

HDDS-8253. Fix checkstyle issue

71694af

github-actions bot removed the pending label Apr 18, 2023

adoroszlai approved these changes Apr 18, 2023

View reviewed changes

adoroszlai changed the title ~~HDDS-8253. Set ozone.metadata.dirs if not defined to tmp dir for s3Gateway~~ HDDS-8253. Set ozone.metadata.dirs to temporary dir if not defined in S3 Gateway Apr 18, 2023

adoroszlai merged commit dd489c7 into apache:master Apr 18, 2023

	String baseDir = conf.get(OzoneConfigKeys.OZONE_HTTP_BASEDIR);

	if (StringUtils.isEmpty(baseDir)) {
	baseDir = getOzoneMetaDirPath(conf) + SERVER_DIR;
	}
	createDir(baseDir);
	httpServer.getWebAppContext().setAttribute(JETTY_BASETMPDIR, baseDir);

HDDS-8253. Set ozone.metadata.dirs to temporary dir if not defined in S3 Gateway #4455

HDDS-8253. Set ozone.metadata.dirs to temporary dir if not defined in S3 Gateway #4455

Uh oh!

Conversation

swamirishi commented Mar 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

Uh oh!

kerneltime commented Mar 23, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

adoroszlai Mar 28, 2023

Choose a reason for hiding this comment

Uh oh!

kerneltime Apr 11, 2023

Choose a reason for hiding this comment

Uh oh!

swamirishi Apr 17, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

adoroszlai left a comment

Choose a reason for hiding this comment

Uh oh!

adoroszlai commented Mar 30, 2023

Uh oh!

neils-dev Apr 3, 2023

Choose a reason for hiding this comment

Uh oh!

swamirishi Apr 17, 2023

Choose a reason for hiding this comment

Uh oh!

neils-dev commented Apr 11, 2023

Uh oh!

adoroszlai commented Apr 11, 2023

Uh oh!

adoroszlai commented Apr 17, 2023

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

adoroszlai left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

adoroszlai commented Apr 17, 2023

Uh oh!

adoroszlai commented Apr 18, 2023

Uh oh!

adoroszlai commented Apr 18, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

swamirishi commented Mar 23, 2023 •

edited

Loading