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

Add documentation for adaptive partitioning and adaptive task sizing of FTE #17179

Merged
merged 4 commits into from
Apr 24, 2023

Conversation

linzebing
Copy link
Member

Description

Add documentation for adaptive partitioning and adaptive task sizing of FTE

Additional context and related issues

Add docs for #16719 and #17024

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:

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

@@ -227,27 +227,17 @@ properties only apply to a ``TASK`` retry policy.
* - Property name
- Description
- Default value
* - ``fault-tolerant-execution-target-task-input-size``
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the behavior for users who still have this (and other removed properties) configured?

Copy link
Member Author

Choose a reason for hiding this comment

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

server startup will fail. Let me add this property to DefunctConfigs

docs/src/main/sphinx/admin/fault-tolerant-execution.rst Outdated Show resolved Hide resolved
docs/src/main/sphinx/admin/fault-tolerant-execution.rst Outdated Show resolved Hide resolved
docs/src/main/sphinx/admin/fault-tolerant-execution.rst Outdated Show resolved Hide resolved
* - ``fault-tolerant-execution-arbitrary-distribution-compute-task-target-size-growth-factor``
- Growth factor for adaptive sizing of non-writer tasks of arbitrary
distribution for fault-tolerant execution.
- ``1.2``
Copy link
Member

Choose a reason for hiding this comment

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

Any upper or lower bounds? Also what does this mean .. explain "growth factor" ... I think it means you add 20% each time .. right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Lower bound is 1.0. Thanks for the reminder, I have added a commit to enforce this check.

think it means you add 20% each time .. right?

Yes. Let me add more color around this one.

docs/src/main/sphinx/admin/fault-tolerant-execution.rst Outdated Show resolved Hide resolved
docs/src/main/sphinx/admin/fault-tolerant-execution.rst Outdated Show resolved Hide resolved
@github-actions github-actions bot added the docs label Apr 21, 2023
@linzebing
Copy link
Member Author

Failure is not related.

Copy link
Contributor

@jhlodin jhlodin left a comment

Choose a reason for hiding this comment

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

Couple more comments, then LGTM

docs/src/main/sphinx/admin/fault-tolerant-execution.rst Outdated Show resolved Hide resolved
docs/src/main/sphinx/admin/fault-tolerant-execution.rst Outdated Show resolved Hide resolved
docs/src/main/sphinx/admin/fault-tolerant-execution.rst Outdated Show resolved Hide resolved
docs/src/main/sphinx/admin/fault-tolerant-execution.rst Outdated Show resolved Hide resolved
Copy link
Member

@colebow colebow left a comment

Choose a reason for hiding this comment

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

Extremely tiny nit, but LGTM. May also require wrapping onto the next line.

* **Default value:** ``false``
* **Session property:** ``determine_partition_count_for_write_enabled``

Enables determining the number of partitions based on amount of data read and processed by the
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Enables determining the number of partitions based on amount of data read and processed by the
Enables determining the number of partitions based on the amount of data read and processed by the

Copy link
Member

@mosabua mosabua left a comment

Choose a reason for hiding this comment

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

This looks good now. Thank you @linzebing .. we will follow up with further improvements as discussed in a separate PR.

@mosabua mosabua merged commit f65be83 into trinodb:master Apr 24, 2023
@github-actions github-actions bot added this to the 415 milestone Apr 25, 2023
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