Skip to content

Use Random instead of UUID to generate randomized prefix for echange spooling#12230

Merged
arhimondr merged 1 commit intotrinodb:masterfrom
linzebing:randomized-prefix
May 4, 2022
Merged

Use Random instead of UUID to generate randomized prefix for echange spooling#12230
arhimondr merged 1 commit intotrinodb:masterfrom
linzebing:randomized-prefix

Conversation

@linzebing
Copy link
Copy Markdown
Member

Description

Is this change a fix, improvement, new feature, refactoring, or other?

Improvement

Is this a change to the core query engine, a connector, client library, or the SPI interfaces? (be specific)

trino-exchange-filesystem

How would you describe this change to a non-technical end user or system administrator?

Improves the performance of randomized prefix generation.

Related issues, pull requests, and links

Documentation

(x) No documentation is needed.
( ) Sufficient documentation is included in this PR.
( ) Documentation PR is available with #prnumber.
( ) Documentation issue #issuenumber is filed, and can be handled later.

Release notes

(x) No release notes entries required.
( ) Release notes entries required with the following suggested text:

# Section
* Fix some things. ({issue}`issuenumber`)

@cla-bot cla-bot bot added the cla-signed label May 3, 2022
@linzebing linzebing requested review from arhimondr and losipiuk May 3, 2022 20:40
@martint
Copy link
Copy Markdown
Member

martint commented May 3, 2022

Please add a note to the commit message describing the motivation.

@linzebing linzebing force-pushed the randomized-prefix branch 4 times, most recently from 135596d to 70b290e Compare May 3, 2022 22:26
Copy link
Copy Markdown
Contributor

@arhimondr arhimondr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM % comment

We want to take advantage of the 36 alphabetic/numeric chars, such that the first two chars has as many combinations as possible.
@linzebing linzebing force-pushed the randomized-prefix branch from 70b290e to f44d0a6 Compare May 3, 2022 23:05
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.ThreadLocalRandom;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from cmt msg

such that the first two chars has as many combinations as possible.

why?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah already merged. This is mainly for S3. Users can talk to AWS to set up manual partitioning for S3 buckets, and they do that base on the first n chars. In our case, it's the first 2.

Copy link
Copy Markdown
Member

@losipiuk losipiuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM % commit message

@arhimondr arhimondr merged commit 15cefaf into trinodb:master May 4, 2022
@github-actions github-actions bot added this to the 380 milestone May 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

5 participants