github-actions[bot] commented on code in PR #62023:
URL: https://github.com/apache/doris/pull/62023#discussion_r3025607665
##########
fe/fe-core/src/main/java/org/apache/doris/backup/Repository.java:
##########
@@ -201,19 +221,20 @@ public static Repository read(DataInput in) throws
IOException {
@Override
public void gsonPostProcess() {
- try {
- StorageProperties storageProperties =
StorageProperties.createPrimary(this.fileSystem.properties);
- this.fileSystem = FileSystemFactory.get(storageProperties);
- } catch (RuntimeException exception) {
- LOG.warn("File system initialization failed due to incompatible
configuration parameters. "
- + "The system has reverted to BrokerFileSystem as
a fallback. "
- + "However, the current configuration is not
supported by this version and"
- + " cannot be used as is. "
- + "Please review the configuration and update it
to match the supported parameter"
- + " format. Root cause: {}",
- ExceptionUtils.getRootCause(exception), exception);
- BrokerProperties brokerProperties =
BrokerProperties.of(this.fileSystem.name, this.fileSystem.properties);
- this.fileSystem = FileSystemFactory.get(brokerProperties);
Review Comment:
Existing repository metadata is serialized with the legacy `"fs"` field, but
this refactor removes that field and `gsonPostProcess()` now just returns when
`fileSystemDescriptor` is null. On upgrade, previously persisted repositories
will deserialize without rebuilding any filesystem state, and later calls like
`getInfo()`, `getCreateStatement()`, `upload()`, or `download()` dereference
`fileSystemDescriptor` and fail. This is an incompatible persistence change in
a replay path, so we need a legacy read/translation path from the old `fs`
payload before removing the old field.
##########
fe/fe-core/src/main/java/org/apache/doris/backup/Repository.java:
##########
@@ -408,23 +489,33 @@ public Status listSnapshots(List<String> snapshotNames) {
// eg. __palo_repository_repo_name/__ss_*
String listPath = Joiner.on(PATH_DELIMITER).join(location,
joinPrefix(PREFIX_REPO, name), PREFIX_SNAPSHOT_DIR)
+ "*";
- List<RemoteFile> result = Lists.newArrayList();
- Status st = fileSystem.globList(listPath, result);
- if (!st.ok()) {
- return st;
+ org.apache.doris.filesystem.FileSystem fs;
+ try {
+ fs = acquireSpiFs();
+ } catch (IOException e) {
+ return new Status(ErrCode.COMMON_ERROR, "Failed to acquire
filesystem: " + e.getMessage());
Review Comment:
The old code used `globList()` for repository discovery, but this switches
to plain `list()/listFiles()` on paths that still contain `*` (`listSnapshots`,
`download`, and `getSnapshotInfo`). The SPI `FileSystem` contract does not
define glob semantics for `list`; for example, `S3FileSystem.list()` treats the
whole string as a literal prefix, while only the broker implementation
explicitly supports globs. That means non-broker repositories can no longer
discover snapshots or checksum-suffixed files. Please keep using a real
glob-capable path here (or a shared helper that expands wildcards) instead of
passing glob patterns to `list`.
--
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]
