HDDS-7495. Create OBS buckets by default from S3 API #3967

errose28 · 2022-11-16T01:07:31Z

What changes were proposed in this pull request?

Currently the create bucket s3 API creates buckets using the server default layout. Since ofs creates FSO buckets, it would make sense for s3 to create OBS buckets.

This is added as a config key in this PR that defaults to OBS. An extra check is added to the s3 gateway so it will continue to work when OM is pre-finalized for bucket layouts.

What is the link to the Apache JIRA

HDDS-7495

How was this patch tested?

TestOzoneRpcClientAbstract#testCreateS3Bucket was modified to check the new layout.
Full upgrade/downgrade test matrix passed on my fork

sadanand48 · 2022-11-16T06:26:56Z

We could add a separate config key to set a default bucket layout for s3g independent of the OM default bucket layout if reviewers think that would be a good thing to have, but the PR does not have this currently

Yup this makes sense for use cases where one can ingest via S3 api and perform FS ops as performing filesystem operations on OBS buckets are currently unsupported. I think we should have such a config.

Also S3 bucket creates during pre-finalize will fail as the request would contain OBJECT_STORE layout and the pre-finalize validator will reject the request , having a config will help here too.

errose28 · 2022-11-16T20:17:57Z

Good catch on the pre-finalize case @sadanand48. I think the best thing to do here is introduce an s3 config for default bucket type to create from s3 gateway like you mentioned. The config will have no default value, and If this is not specified, then s3g will not send a bucket layout type causing the OM default type to be used. Here the OM can use LEGACY if it is pre-finalized.

I am not sure there is a way to force s3g to use OBS buckets out of the box since it does not have the OM's pre-finalized state. At least by adding this new config, legacy buckets will not be created by default after #3966, and users can make s3g create OBS buckets if they want.

#3966 will be making FSO the default bucket layout type on OM, and I will add handling there to use LEGACY buckets if OM is pre-finalized, regardless of the config value. For this reason I think this PR depends on that one, so we can pause here until that is finished.

* master: (209 commits) HDDS-7097. Container scanner log output lacks useful information (apache#4169) HDDS-7813. Handle Mismatched Replicas (OPEN or CLOSING) of QUASI-CLOSED containers in RM (apache#4195) HDDS-7625. Do not compress OM/SCM checkpoints (apache#4130) HDDS-7801. Bucket not found when calling getKeyInfo with tenant context (apache#4189) HDDS-7807. TarContainerPacker closes streams multiple times (apache#4193) HDDS-7755. Ensure that acquired locks are always released. (apache#4191) HDDS-7804. UNHEALTHY replicas will not contribute to sufficient replication in RatisContainerReplicaCount (apache#4192) HDDS-7748. Rename OMFileRequest.addToOpenFileTable() to avoid misuse. (apache#4176) HDDS-7723. Refresh Keys and Certificate used in OzoneSecretManager after certificate renewed (apache#4179) HDDS-7788. Ratis OverReplicationHandler should exclude stale replicas (apache#4183) HDDS-7718. Bump Netty to 4.1.86 and gRPC to 1.51.1 (apache#4139) HDDS-7542. Refactor DefaultReplicationConfig (apache#4005) HDDS-7787. GetChecksum for EC files can fail intermittently with IndexOutOfBounds exception (apache#4180) HDDS-7754. Download of container is failing with SSL/TLS error during re-replication (apache#4174) HDDS-7455. ClassCastException: OzoneTokenIdentifier cannot be cast to String (apache#4159) HDDS-7441. Rename function names of retrieving metadata keys (apache#3918) HDDS-7722. FSO buckets fail to invalidate open file table cache when committing a key (apache#4156) HDDS-7774. Update outdated Trash documentation (apache#4172) HDDS-7761. EC: ReplicationManager - Use placementPolicy.replicasToRemoveToFixOverreplication in EC Over replication handler (apache#4166) HDDS-7775. EC: Exception encountered while deleting UNHEALTHY replica in Datanode (apache#4173) ...

DaveTeng0 · 2023-01-30T04:54:50Z

@errose28 Hey ethan~ there're some tests failing. Let's resolve them!

kerneltime · 2023-04-17T07:31:08Z

This change looks good to deal with the upgrade scenario; S3G can retry the bucket creation with LEGACY if the exception allows S3G to identify the condition correctly. It will be a lot simpler than adding complex upgrade scenarios and relevant testing for a case that will be rare. It would make sense to allow the bucket type for S3G to be a config for S3G. This will enable situations where a customer wants to revert to LEGACY for compatibility with the old behavior for buckets and for customers such as the use case mentioned HDDS-7701 to create FSO buckets.

kerneltime · 2023-04-17T07:39:28Z

Updated the doc: https://docs.google.com/document/d/1wVlbJX22yw84WowH6I4ni_pUvaxKDr9JLHHsEVOTYSA/edit#heading=h.qme9ukoq72k

* 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) ...

hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java

Set s3 bucket layout in ctor Use vars in log message

arp7 · 2023-05-01T15:39:49Z

@kerneltime does this look good to you?

arp7

LGTM

adoroszlai

LGTM, two minor comments, address only if any other change is required.

adoroszlai · 2023-05-07T07:29:43Z

hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java


-  public static final String OZONE_CLIENT_FS_BUCKET_LAYOUT_LEGACY =


Should we keep this constant for binary backward compatibility?

adoroszlai · 2023-05-07T07:35:34Z

hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java

+      throw new ConfigurationException(bucketLayoutString +
+          " is not a valid default bucket layout. Supported values are " +
+          Arrays.stream(BucketLayout.values())
+              .map(Enum::toString).collect(Collectors.joining(", ")));


Nit: validity check above uses Enum#name(), but exception message uses toString(). These are equivalent by default, but toString() can be overridden (e.g. for i18n/l10n purposes).

Suggested change

.map(Enum::toString).collect(Collectors.joining(", ")));

.map(Enum::name).collect(Collectors.joining(", ")));

Better yet, create a SortedSet<String> constant in BucketLayout with all names, and use it in both validity check and exception message.

arp7 · 2023-05-10T18:37:12Z

Let's address @adoroszlai's comments asap in a follow up patch.

Make OBS default layout from s3 and add test

72ea80d

errose28 requested a review from sadanand48 November 16, 2022 01:07

errose28 mentioned this pull request Nov 16, 2022

HDDS-7496. Make default bucket layout FSO in a backwards compatible way. #3966

Merged

kerneltime self-requested a review January 23, 2023 17:21

errose28 added 2 commits January 24, 2023 12:57

Add config for s3g bucket layout type

4dc3069

errose28 marked this pull request as draft April 20, 2023 18:17

errose28 added 3 commits April 20, 2023 11:28

Backwards compatibility for bucket layout

f78c912

Add new config to ozone-default.xml

5704de1

kerneltime reviewed Apr 20, 2023

View reviewed changes

hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java Outdated Show resolved Hide resolved

kerneltime reviewed Apr 20, 2023

View reviewed changes

hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/ObjectStore.java Show resolved Hide resolved

Address review comments

83c36d1

Set s3 bucket layout in ctor Use vars in log message

errose28 marked this pull request as ready for review April 20, 2023 23:19

errose28 requested a review from kerneltime April 20, 2023 23:20

Update TODO now that S3 creates OBS by default.

cbc019f

arp7 approved these changes May 4, 2023

View reviewed changes

adoroszlai reviewed May 7, 2023

View reviewed changes

kerneltime approved these changes May 10, 2023

View reviewed changes

arp7 merged commit ba1bd23 into apache:master May 10, 2023
27 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HDDS-7495. Create OBS buckets by default from S3 API #3967

HDDS-7495. Create OBS buckets by default from S3 API #3967

errose28 commented Nov 16, 2022 •

edited

sadanand48 commented Nov 16, 2022 •

edited

errose28 commented Nov 16, 2022

DaveTeng0 commented Jan 30, 2023

kerneltime commented Apr 17, 2023 •

edited

kerneltime commented Apr 17, 2023

arp7 commented May 1, 2023

arp7 left a comment

adoroszlai left a comment

adoroszlai May 7, 2023

adoroszlai May 7, 2023

arp7 commented May 10, 2023


		public static final String OZONE_CLIENT_FS_BUCKET_LAYOUT_LEGACY =

	.map(Enum::toString).collect(Collectors.joining(", ")));
	.map(Enum::name).collect(Collectors.joining(", ")));

HDDS-7495. Create OBS buckets by default from S3 API #3967

HDDS-7495. Create OBS buckets by default from S3 API #3967

Conversation

errose28 commented Nov 16, 2022 • edited

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

sadanand48 commented Nov 16, 2022 • edited

errose28 commented Nov 16, 2022

DaveTeng0 commented Jan 30, 2023

kerneltime commented Apr 17, 2023 • edited

kerneltime commented Apr 17, 2023

arp7 commented May 1, 2023

arp7 left a comment

Choose a reason for hiding this comment

adoroszlai left a comment

Choose a reason for hiding this comment

adoroszlai May 7, 2023

Choose a reason for hiding this comment

adoroszlai May 7, 2023

Choose a reason for hiding this comment

arp7 commented May 10, 2023

errose28 commented Nov 16, 2022 •

edited

sadanand48 commented Nov 16, 2022 •

edited

kerneltime commented Apr 17, 2023 •

edited