-
Notifications
You must be signed in to change notification settings - Fork 3.8k
[opt](s3) Skip S3 listing for deterministic file paths using HEAD requests (#60414) #61775
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: branch-4.0
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -18,6 +18,7 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| package org.apache.doris.fs.obj; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.apache.doris.backup.Status; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.apache.doris.common.Config; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.apache.doris.common.DdlException; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.apache.doris.common.UserException; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.apache.doris.common.util.S3URI; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -357,8 +358,24 @@ public Status globList(String remotePath, List<RemoteFile> result, boolean fileN | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| remotePath = AzurePropertyUtils.validateAndNormalizeUri(remotePath); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| S3URI uri = S3URI.create(remotePath, isUsePathStyle, forceParsingByStandardUri); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| String globPath = S3Util.extendGlobs(uri.getKey()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| String bucket = uri.getBucket(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Optimization: For deterministic paths (no wildcards like *, ?), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // use getProperties requests instead of listing to avoid requiring list permission. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Controlled by config: s3_skip_list_for_deterministic_path | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Note: Skip when using path style (see S3ObjStorage for detailed explanation) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| String keyPattern = uri.getKey(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (Config.s3_skip_list_for_deterministic_path | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| && !isUsePathStyle | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| && S3Util.isDeterministicPattern(keyPattern)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Status headStatus = globListByGetProperties(bucket, keyPattern, result, fileNameOnly, startTime); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (headStatus != null) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return headStatus; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // If headStatus is null, fall through to use listing | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+363
to
+376
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| String globPath = S3Util.extendGlobs(uri.getKey()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (LOG.isDebugEnabled()) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| LOG.debug("try to glob list for azure, remote path {}, orig {}", globPath, remotePath); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -436,6 +453,86 @@ public Status globList(String remotePath, List<RemoteFile> result, boolean fileN | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return st; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Get file metadata using getProperties requests for deterministic paths. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * This avoids requiring list permission when only read permission is granted. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param bucket Azure container name | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param keyPattern The key pattern (may contain {..} brace or [...] bracket patterns but no wildcards) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param result List to store matching RemoteFile objects | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param fileNameOnly If true, only store file names; otherwise store full paths | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @param startTime Start time for logging duration | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @return Status if successful, null if should fall back to listing | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private Status globListByGetProperties(String bucket, String keyPattern, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| List<RemoteFile> result, boolean fileNameOnly, long startTime) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // First expand [...] brackets to {...} braces, then expand {..} ranges, then expand braces | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| String expandedPattern = S3Util.expandBracketPatterns(keyPattern); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expandedPattern = S3Util.extendGlobs(expandedPattern); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| List<String> expandedPaths = S3Util.expandBracePatterns(expandedPattern); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Fall back to listing if too many paths to avoid overwhelming Azure with requests | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Controlled by config: s3_head_request_max_paths | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (expandedPaths.size() > Config.s3_head_request_max_paths) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| LOG.info("Expanded path count {} exceeds limit {}, falling back to LIST", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expandedPaths.size(), Config.s3_head_request_max_paths); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+470
to
+480
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // First expand [...] brackets to {...} braces, then expand {..} ranges, then expand braces | |
| String expandedPattern = S3Util.expandBracketPatterns(keyPattern); | |
| expandedPattern = S3Util.extendGlobs(expandedPattern); | |
| List<String> expandedPaths = S3Util.expandBracePatterns(expandedPattern); | |
| // Fall back to listing if too many paths to avoid overwhelming Azure with requests | |
| // Controlled by config: s3_head_request_max_paths | |
| if (expandedPaths.size() > Config.s3_head_request_max_paths) { | |
| LOG.info("Expanded path count {} exceeds limit {}, falling back to LIST", | |
| expandedPaths.size(), Config.s3_head_request_max_paths); | |
| return null; | |
| // First expand [...] brackets to {...} braces, then expand {..} ranges. | |
| // NOTE: We intentionally avoid unbounded brace expansion here to prevent | |
| // potential CPU/memory exhaustion on very large patterns. | |
| String expandedPattern = S3Util.expandBracketPatterns(keyPattern); | |
| expandedPattern = S3Util.extendGlobs(expandedPattern); | |
| final List<String> expandedPaths; | |
| if (expandedPattern.contains("{")) { | |
| // Pattern still contains brace/range expressions. To avoid potentially | |
| // huge expansions, conservatively fall back to LIST-based globbing. | |
| LOG.info("Pattern '{}' contains brace/range expressions after normalization; " | |
| + "skipping deterministic getProperties optimization and falling back to LIST", | |
| expandedPattern); | |
| return null; | |
| } else { | |
| // Deterministic single path; no brace expansion needed. | |
| expandedPaths = new ArrayList<>(1); | |
| expandedPaths.add(expandedPattern); |
Copilot
AI
Mar 26, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RemoteFile modification times elsewhere in FE are generally in epoch milliseconds (e.g. HDFS FileStatus#getModificationTime(), S3 toEpochMilli()), and FileGroupInfo passes this through to scan ranges. Using toEpochSecond() here will make Azure deterministic-path results inconsistent and likely incorrect by 1000x. Prefer props.getLastModified().toInstant().toEpochMilli() (and consider aligning the existing Azure LIST path too).
| ? props.getLastModified().toEpochSecond() : 0 | |
| ? props.getLastModified().toInstant().toEpochMilli() : 0 |
Copilot
AI
Mar 26, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fallback log drops the exception stack trace (LOG.warn(..., e.getMessage())), which makes it hard to understand why getProperties failed (permission vs missing blob vs transient errors). Log the throwable (and include container/keyPattern) so operational debugging is possible.
| LOG.warn("Failed to use getProperties requests, falling back to listing: {}", e.getMessage()); | |
| LOG.warn("Failed to use getProperties requests for container '{}' and keyPattern '{}', " | |
| + "falling back to listing", bucket, keyPattern, e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
expandBracePatterns()always fully expands into aListwith no way to cap the number of generated paths. Since this is used to guard HEAD/getProperties fan-out, consider adding a limit-aware variant (e.g.expandBracePatterns(pattern, maxPaths)) or stopping recursion once a threshold is exceeded, to avoid large allocations on patterns that will later be rejected.