[#6562] improvement(catalogs): Optimize the code path in createFileset and optimize path

[Abyss-lord]

What changes were proposed in this pull request?

Optimize the code path in createFileset and optimize path by using future.

Why are the changes needed?

Fix: #6562

Does this PR introduce any user-facing change?

No

How was this patch tested?

No， this is sequence chart of the CreateFileset.

[Abyss-lord]

@yuqi1129 could you please review this PR when you have time? I’d really appreciate your feedback.

[yuqi1129]

@yuqi1129 could you please review this PR when you have time? I’d really appreciate your feedback.

Sure

[Abyss-lord]

@yuqi1129 I’ve finished updating the code. Please take a look at the PR again when you have time. I make thread pool static and remove shutdown logic in finally block.

[yuqi1129]

Some minor problems, others LGTM

[Abyss-lord]

@yuqi1129 I’ve finished updating the code. Please take a look at the PR again when you have time. My update is as follows

1. Set the number of core threads and queue size.
2. Set the idle time.

[Abyss-lord]

@yuqi1129 I’ve finished updating the code. Please take a look at the PR again when you have time. My update is as follows

1. set queue size from 100 to 500

[Abyss-lord]

@jerqi I’ve finished updating the code. Please take a look at the PR again when you have time. My update is as follows

1. set min thread to Math.max(Math.min(Runtime.getRuntime().availableProcessors() * 2, 100), 4)
2. set max thread to Math.max(Runtime.getRuntime().availableProcessors() * 4, 400).

[yuqi1129]

Let's wait for the CI to pass before proceeding with LGTM.

[yuqi1129]

@Abyss-lord
There seems to be an issue with the CI. Please take some time to resolve it.

[Abyss-lord]

@Abyss-lord There seems to be an issue with the CI. Please take some time to resolve it.

@yuqi1129 This problem is probably not caused by this change. The same situation occurred before, and it can be solved by running again.

[yuqi1129]

@Abyss-lord There seems to be an issue with the CI. Please take some time to resolve it.

@yuqi1129 This problem is probably not caused by this change. The same situation occurred before, and it can be solved by running again.

Although things may not be as they seem, I have noticed that no similar problem has been found in other PRs.

[yuqi1129]

@Abyss-lord There seems to be an issue with the CI. Please take some time to resolve it.

@yuqi1129 This problem is probably not caused by this change. The same situation occurred before, and it can be solved by running again.

Although things may not be as they seem, I have noticed that no similar problem has been found in other PRs.

@Abyss-lord
There are some test errors that can be reproduced locally. Please try to resolve them.

[jerryshao]

What is the benefit of this change? I think using threadpool may not benefit a lot, instead it makes the code very complex. Also, we may potentially have the classloader issue since we're using the different thread to load the FS.

[yuqi1129]

What is the benefit of this change? I think using threadpool may not benefit a lot, instead it makes the code very complex. Also, we may potentially have the classloader issue since we're using the different thread to load the FS.

The awaitiblity library will periodically poll the result and will not return immediately when the condition is satisfied.

Awaitility.await()
          .atMost(timeoutSeconds, TimeUnit.SECONDS)
          .pollInterval(TimeUnits.MilliSeconds, 1)
          .until(
				() -> {
                            fileSystem.set(provider.getFileSystem(path, config));
                            return true;
                         }
                  )

Even though fileSystem.set(provider.getFileSystem(path, config)); takes 0ms, the whole code will mostly consume 1ms at least to get the result. Java Future will not have this problem.

Actually, introducing thread pools will make things a bit complicated. If the issue mentioned above is not a big problem, we can omit this PR.

[Abyss-lord]

@yuqi1129 @jerryshao Should I continue with this pr, or should we pause to discuss?

[yuqi1129]

@yuqi1129 @jerryshao Should I continue with this pr, or should we pause to discuss?

OK, please revert the code about thread-pool and only change the poll interval of awaitibility