Skip to content
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

[MRESOLVER-554] Split thread counts for up and downstream ops #489

Merged
merged 4 commits into from May 2, 2024

Conversation

cstamas
Copy link
Member

@cstamas cstamas commented Apr 30, 2024

Basic connector new config to be able to differentiate thread counts used for upload and download.


https://issues.apache.org/jira/browse/MRESOLVER-554

Basic connector new config to be able to differentiate thread
counts used for upload and download.

---

https://issues.apache.org/jira/browse/MRESOLVER-554
@cstamas cstamas self-assigned this Apr 30, 2024
@cstamas cstamas marked this pull request as ready for review April 30, 2024 20:04
Copy link
Member

@michael-o michael-o left a comment

Choose a reason for hiding this comment

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

All fine, just a nit.


private final boolean smartChecksums;

private final boolean parallelPut;

private final boolean persistedChecksums;

private Executor executor;
private final ConcurrentHashMap<Boolean, Executor> executors;
Copy link
Member

Choose a reason for hiding this comment

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

Just a remark. An Enum would read better than just a boolean...

Copy link
Member

Choose a reason for hiding this comment

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

Enum will be more descriptive in methods calls than only simply true or false as parameter.

Copy link
Member Author

@cstamas cstamas May 2, 2024

Choose a reason for hiding this comment

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

I disagree here, here is why: if I add enum, it will complicate method a lot, will need to null-check it, will need to throw IAEx if not expected (default branch of switch), etc... with boolean there is no need for null-check and can really be this or that. IF we have need to keep more than 2 executors for any reason, this can be easily extended to enum, but right now I see no point of doing it.

Copy link

Choose a reason for hiding this comment

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

with boolean there is no need for null-check and can really be this or that.

Just a remark, a Boolean can be null ... beside that ConcurrentHashMap does not allows null keys...

@cstamas cstamas merged commit b5f8388 into apache:master May 2, 2024
4 checks passed
@cstamas cstamas deleted the MRESOLVER-554 branch May 2, 2024 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants