[Feature] S3 storage volume support partitioned prefix feature #41627

kevincai · 2024-02-26T07:05:15Z

s3 storage volume added the following two properties to support partitioned prefix feature
- aws.s3.enable_partitioned_prefix
- aws.s3.num_partitioned_prefix

Why I'm doing:

What I'm doing:

Fixes #issue

What type of PR is this:

Does this PR entail a change in behavior?

Yes, this PR will result in a change in behavior.
No, this PR will not result in a change in behavior.

If yes, please specify the type of change:

Interface/UI changes: syntax, type conversion, expression evaluation, display information
Parameter changes: default values, similar parameters but with different default values
Policy changes: use new policy to replace old one, functionality automatically enabled
Feature removed
Miscellaneous: upgrade & downgrade compatibility, etc.

Checklist:

I have added test cases for my bug fix or my new feature
This pr needs user documentation (for new or modified features or behaviors)
- I have added documentation for my new feature or new function
This is a backport pr

Bugfix cherry-pick branch check:

fe/fe-core/src/main/java/com/starrocks/storagevolume/StorageVolume.java

fe/fe-core/src/main/java/com/starrocks/server/StorageVolumeMgr.java

fe/fe-core/src/main/java/com/starrocks/credential/aws/AWSCloudConfiguration.java

starrocks-xupeng · 2024-02-26T11:30:53Z

fe/fe-core/src/main/java/com/starrocks/credential/aws/AWSCloudConfiguration.java

+            if (value != null) {
+                try {
+                    int val = Integer.parseInt(value);
+                    if (val < 0) {


deal with 0

0 is handled as if the value is not provided, which is in line 96.

starrocks-xupeng · 2024-02-26T11:31:18Z

fe/fe-core/src/main/java/com/starrocks/credential/aws/AWSCloudConfiguration.java

+                        this.numOfPartitionedPrefix = val;
+                    }
+                } catch (NumberFormatException e) {
+                    throw new IllegalArgumentException(


can upper level handle this exception?

yes, it can, refer to the UT.

starrocks-xupeng · 2024-02-26T11:35:34Z

fe/fe-core/src/main/java/com/starrocks/storagevolume/StorageVolume.java

+        validateStorageVolumeConstraints();
+    }
+
+    private void validateStorageVolumeConstraints() throws DdlException {


seems not necessary?

subpath is not allowed when storage volume is enabled with partitioned prefix, otherwise the meaning of subpath is confusing.

* s3 storage volume added the following two properties to support partitioned prefix feature - aws.s3.enable_partitioned_prefix - aws.s3.num_partitioned_prefix Signed-off-by: Kevin Xiaohua Cai <caixiaohua@starrocks.com>

sonarcloud · 2024-02-26T15:41:42Z

Quality Gate passed

Issues
3 New issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

github-actions · 2024-02-26T17:17:33Z

[BE Incremental Coverage Report]

✅ pass : 0 / 0 (0%)

github-actions · 2024-02-26T17:17:33Z

[FE Incremental Coverage Report]

✅ pass : 74 / 75 (98.67%)

file detail

	path	covered_line	new_line	coverage	not_covered_line_detail
🔵	com/starrocks/storagevolume/StorageVolume.java	24	25	96.00%	[129]
🔵	com/starrocks/credential/aws/AWSCloudConfiguration.java	25	25	100.00%	[]
🔵	com/starrocks/catalog/OlapTable.java	1	1	100.00%	[]
🔵	com/starrocks/server/StorageVolumeMgr.java	22	22	100.00%	[]
🔵	com/starrocks/lake/StarOSAgent.java	2	2	100.00%	[]

TszKitLo40 · 2024-02-28T01:40:16Z

fe/fe-core/src/main/java/com/starrocks/server/StorageVolumeMgr.java

@@ -164,6 +165,14 @@ public void updateStorageVolume(String name, Map<String, String> params, Optiona
            StorageVolume copied = new StorageVolume(sv);
            validateParams(copied.getType(), params);

+            List<String> immutableProperties =


How about making immutableProperties as a member of StorageVolumeMgr?

yes it can. However, I am thinking about refactor the StorageVolume, derive it by S3StorageVolume/HdfsStorageVolume/AzblobStorageVolume, each storage volume can handle its own immutable properties and validation.

It's beyond this feature, so in this PR, just keep the simplest coding, refactor it in a separate PR.

Smith-Cruise · 2024-02-28T03:03:53Z

fe/fe-core/src/main/java/com/starrocks/credential/aws/AWSCloudConfiguration.java

    }

    @Override
    public String toConfString() {
+        // TODO: add enable_partitioned_prefix, num_partitioned_prefix output


why todo, we will use toConfString as map's key.

these two properties are specific for storage volume, won't be used for external table property configuration. So add todo here.

kevincai · 2024-02-28T03:21:39Z

can you explain about aws.s3.enable_partitioned_prefix aws.s3.num_partitioned_prefix

sent you the doc link

kevincai · 2024-02-28T03:54:55Z

@Mergifyio backport branch-3.2

mergify · 2024-02-28T03:54:59Z

backport branch-3.2

✅ Backports have been created

#41821 [Feature] S3 storage volume support partitioned prefix feature (backport #41627) has been created for branch branch-3.2

Signed-off-by: Kevin Xiaohua Cai <caixiaohua@starrocks.com> (cherry picked from commit de98d87)

Signed-off-by: Kevin Xiaohua Cai <caixiaohua@starrocks.com> (cherry picked from commit de98d87) Signed-off-by: Kevin Xiaohua Cai <caixiaohua@starrocks.com>

…ort #41627) (#41821) Signed-off-by: Kevin Xiaohua Cai <caixiaohua@starrocks.com> Co-authored-by: Kevin Cai <caixiaohua@starrocks.com>

…ocks#41627) Signed-off-by: Kevin Xiaohua Cai <caixiaohua@starrocks.com> Signed-off-by: Seaven <seaven_7@qq.com>

evelynzhaojie · 2024-03-05T08:28:59Z

@kevincai pls notify TW Wanglichen of this change or submit a PR to update the doc

kevincai requested review from a team as code owners February 26, 2024 07:05

github-actions bot added the behavior_changed label Feb 26, 2024

starrocks-cr bot reviewed Feb 26, 2024

View reviewed changes

fe/fe-core/src/main/java/com/starrocks/storagevolume/StorageVolume.java Show resolved Hide resolved

mergify bot assigned kevincai Feb 26, 2024

starrocks-cr bot reviewed Feb 26, 2024

View reviewed changes

fe/fe-core/src/main/java/com/starrocks/server/StorageVolumeMgr.java Show resolved Hide resolved

starrocks-cr bot reviewed Feb 26, 2024

View reviewed changes

fe/fe-core/src/main/java/com/starrocks/credential/aws/AWSCloudConfiguration.java Show resolved Hide resolved

kevincai force-pushed the fe-storage-volume-prefix branch 2 times, most recently from 4f249c9 to 95bd643 Compare February 26, 2024 10:52

sduzh self-requested a review February 26, 2024 11:10

starrocks-xupeng reviewed Feb 26, 2024

View reviewed changes

kevincai force-pushed the fe-storage-volume-prefix branch from 95bd643 to 20f7a84 Compare February 26, 2024 12:35

[Feature] S3 storage volume support partitioned prefix feature

da704d3

* s3 storage volume added the following two properties to support partitioned prefix feature - aws.s3.enable_partitioned_prefix - aws.s3.num_partitioned_prefix Signed-off-by: Kevin Xiaohua Cai <caixiaohua@starrocks.com>

kevincai force-pushed the fe-storage-volume-prefix branch from 20f7a84 to da704d3 Compare February 26, 2024 15:34

sduzh approved these changes Feb 27, 2024

View reviewed changes

kevincai requested review from TszKitLo40 and Smith-Cruise February 27, 2024 23:24

TszKitLo40 reviewed Feb 28, 2024

View reviewed changes

dirtysalt approved these changes Feb 28, 2024

View reviewed changes

TszKitLo40 approved these changes Feb 28, 2024

View reviewed changes

This comment was marked as outdated.

Sign in to view

Smith-Cruise reviewed Feb 28, 2024

View reviewed changes

sduzh approved these changes Feb 28, 2024

View reviewed changes

sduzh enabled auto-merge (squash) February 28, 2024 03:35

sduzh disabled auto-merge February 28, 2024 03:35

nshangyiming approved these changes Feb 28, 2024

View reviewed changes

nshangyiming merged commit de98d87 into StarRocks:main Feb 28, 2024
70 checks passed

mergify bot pushed a commit that referenced this pull request Feb 28, 2024

[Feature] S3 storage volume support partitioned prefix feature (#41627)

62d00f6

Signed-off-by: Kevin Xiaohua Cai <caixiaohua@starrocks.com> (cherry picked from commit de98d87)

mergify bot mentioned this pull request Feb 28, 2024

[Feature] S3 storage volume support partitioned prefix feature (backport #41627) #41821

Merged

18 tasks

github-actions bot added the 3.2-merged label Feb 28, 2024

Seaven pushed a commit to Seaven/starrocks that referenced this pull request Feb 28, 2024

[Feature] S3 storage volume support partitioned prefix feature (StarR…

bac3ebf

…ocks#41627) Signed-off-by: Kevin Xiaohua Cai <caixiaohua@starrocks.com> Signed-off-by: Seaven <seaven_7@qq.com>

kevincai deleted the fe-storage-volume-prefix branch March 5, 2024 08:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feature] S3 storage volume support partitioned prefix feature #41627

[Feature] S3 storage volume support partitioned prefix feature #41627

kevincai commented Feb 26, 2024 •

edited

starrocks-xupeng Feb 26, 2024

kevincai Feb 26, 2024 •

edited

starrocks-xupeng Feb 26, 2024 •

edited

kevincai Feb 26, 2024

starrocks-xupeng Feb 26, 2024

kevincai Feb 26, 2024

sonarcloud bot commented Feb 26, 2024

github-actions bot commented Feb 26, 2024

github-actions bot commented Feb 26, 2024

TszKitLo40 Feb 28, 2024

kevincai Feb 28, 2024

This comment was marked as outdated.

Smith-Cruise Feb 28, 2024

kevincai Feb 28, 2024

kevincai commented Feb 28, 2024

kevincai commented Feb 28, 2024

mergify bot commented Feb 28, 2024 •

edited

evelynzhaojie commented Mar 5, 2024

[Feature] S3 storage volume support partitioned prefix feature #41627

[Feature] S3 storage volume support partitioned prefix feature #41627

Conversation

kevincai commented Feb 26, 2024 • edited

Why I'm doing:

What I'm doing:

What type of PR is this:

Checklist:

Bugfix cherry-pick branch check:

Choose a reason for hiding this comment

kevincai Feb 26, 2024 • edited

Choose a reason for hiding this comment

starrocks-xupeng Feb 26, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Feb 26, 2024

Quality Gate passed

github-actions bot commented Feb 26, 2024

[BE Incremental Coverage Report]

github-actions bot commented Feb 26, 2024

[FE Incremental Coverage Report]

file detail

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as outdated.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kevincai commented Feb 28, 2024

kevincai commented Feb 28, 2024

mergify bot commented Feb 28, 2024 • edited

✅ Backports have been created

evelynzhaojie commented Mar 5, 2024

kevincai commented Feb 26, 2024 •

edited

kevincai Feb 26, 2024 •

edited

starrocks-xupeng Feb 26, 2024 •

edited

mergify bot commented Feb 28, 2024 •

edited