-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Disable task level writer scaling from TableExecute #18388
Disable task level writer scaling from TableExecute #18388
Conversation
bb6e2ce
to
29db74d
Compare
core/trino-main/src/main/java/io/trino/sql/planner/optimizations/AddLocalExchanges.java
Outdated
Show resolved
Hide resolved
plugin/trino-hive/src/test/java/io/trino/plugin/hive/BaseHiveConnectorTest.java
Show resolved
Hide resolved
29db74d
to
8536820
Compare
core/trino-main/src/main/java/io/trino/sql/planner/optimizations/AddLocalExchanges.java
Show resolved
Hide resolved
8536820
to
c06d835
Compare
c06d835
to
0f6a774
Compare
plugin/trino-delta-lake/src/test/java/io/trino/plugin/deltalake/TestDeltaLakeBasic.java
Outdated
Show resolved
Hide resolved
...ng/trino-product-tests/src/main/java/io/trino/tests/product/iceberg/TestIcebergOptimize.java
Show resolved
Hide resolved
plugin/trino-iceberg/src/test/java/io/trino/plugin/iceberg/BaseIcebergConnectorTest.java
Outdated
Show resolved
Hide resolved
assertUpdate("INSERT INTO " + tableName + " VALUES 10", 1); | ||
assertUpdate("INSERT INTO " + tableName + " VALUES 20", 1); | ||
assertUpdate("INSERT INTO " + tableName + " VALUES NULL", 1); | ||
assertUpdate("ALTER TABLE " + tableName + " EXECUTE OPTIMIZE"); | ||
assertUpdate(Session.builder(getQueryRunner().getDefaultSession()) | ||
.setSystemProperty("task_writer_count", "1") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of setting this session property, can we instead adjust the assertions for the increased file count ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should explicitly set the value of task_writer_count. This makes the test more reliable.
It can result in smaller files than file_size_threshold, which can be undesirable behaviour.
0f6a774
to
4acf294
Compare
Description
Currently, task writer scaling can lead to file sizes which are not controlled via OPTIMIZE command thus making it unreliable.
Additional context and related issues
Release notes
( ) 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.
(x) Release notes are required, with the following suggested text: