[#5361] improvment(hadoop-catalog): Introduce timeout mechanism to get Hadoop File System. #5406
Conversation
|
@jerryshao |
| .atMost(timeoutSeconds, TimeUnit.SECONDS) | ||
| .until( | ||
| () -> { | ||
| fileSystem.set(provider.getFileSystem(path, config)); |
There was a problem hiding this comment.
Why it is so time-consuming to initialize the filesystem client?
There was a problem hiding this comment.
If the user sets an incorrect endpoint, the client will retry to get the connection for a certain amount of time.
There was a problem hiding this comment.
I don't think you really fix this problem without using another thread to create a FS and polling the status asynchronously?
|
@jerryshao |
yuqi1129
changed the title
|
I will spend time on this. |
Got |
| } catch (ConditionTimeoutException e) { | ||
| throw new IOException( | ||
| String.format( | ||
| "Failed to get FileSystem for path: %s, scheme: %s, provider: %s, config: %s within %s seconds, please check the configuration or increase the " |
There was a problem hiding this comment.
This line exceeds the 100 characters length, please break into lines.
| @@ -53,6 +53,10 @@ public class HadoopCatalogPropertiesMetadata extends BaseCatalogPropertiesMetada | |||
| */ | |||
| public static final String DEFAULT_FS_PROVIDER = "default-filesystem-provider"; | |||
|
|
|||
| static final String FILESYSTEM_CONNECTION_TIMEOUT_SECONDS = | |||
| "filesystem-connection-timeout-seconds"; | |||
| static final int DEFAULT_GET_FILESYSTEM_TIMEOUT_SECONDS = 6; | |||
There was a problem hiding this comment.
Why it is the "6" seconds we defined here?
There was a problem hiding this comment.
This is just an experience value. This specific setting depends on the environment at that time. I believe 6s is long enough to get the connection.
| @@ -53,6 +53,10 @@ public class HadoopCatalogPropertiesMetadata extends BaseCatalogPropertiesMetada | |||
| */ | |||
| public static final String DEFAULT_FS_PROVIDER = "default-filesystem-provider"; | |||
|
|
|||
| static final String FILESYSTEM_CONNECTION_TIMEOUT_SECONDS = | |||
| "filesystem-connection-timeout-seconds"; | |||
There was a problem hiding this comment.
The configuration name is too long, can you make it shorter.
There was a problem hiding this comment.
How about "filesystem-conn-timeout-sec"?
What changes were proposed in this pull request?
Introduce a timeout mechanism when getting a Hadoop FileSystem instance.
Why are the changes needed?
Cloud filesystem like S3 and OSS(10 minutes) has a very long connection and can't be tune by configuration, this will cause deadlock as it will hold the tree lock for a long time
Fix: #5361
Fix: #6156
Does this PR introduce any user-facing change?
N/A.
How was this patch tested?
Existing test.