Enforce limit on FTE max partition count#17876
Conversation
linzebing
force-pushed
the
force-limit-on-partition-count
branch
2 times, most recently
from
f1599d1 to
937f98c
Compare
core/trino-main/src/main/java/io/trino/SystemSessionProperties.java
Outdated
Show resolved
Hide resolved
core/trino-main/src/main/java/io/trino/SystemSessionProperties.java
Outdated
Show resolved
Hide resolved
linzebing
force-pushed
the
force-limit-on-partition-count
branch
from
937f98c to
2432d6e
Compare
There was a problem hiding this comment.
nit: extract constant for 1000
There was a problem hiding this comment.
Also I am not a big fan of hardcoding 1000. It is a bit arbitrary.
Probably it should be limitted to queryManagerConfig.getFaultTolerantExecutionMaxPartitionCount() and we would need an extra config parameter to set default value of session property.
But I do not feel to strongly about that.
There was a problem hiding this comment.
It's weird to have too many configs:) I don't see any similar configs like that, so I extract constant (similar to MAX_TASK_RETRY_ATTEMPTS)
There was a problem hiding this comment.
There are some like that. But yeah - I agree this generates an explosion,.
There was a problem hiding this comment.
MAX_TASK_RETRY_ATTEMPTS is different animal. As with that you just set number of attempts. And here you specify FAULT_TOLERANT_EXECUTION_MAX_PARTITION_COUNT you specify number of partitions and 1000 hard coded limit on what you can set. There is no such hardcoded limit for MAX_TASK_RETRY_ATTEMPTS.
Let's leave 1000 in the code for now. We can revisit later if needed.
core/trino-main/src/main/java/io/trino/execution/QueryManagerConfig.java
Outdated
Show resolved
Hide resolved
linzebing
force-pushed
the
force-limit-on-partition-count
branch
from
2432d6e to
85ec14e
Compare
3 participants
Description
To ensure the session property isn't accidentally set to an overwhelming number.
Additional context and related issues
N/A
Release notes
(x) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text: