-
Notifications
You must be signed in to change notification settings - Fork 2.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
copy editing changes to summary #16880
Conversation
Signed-off-by: deepthi <[email protected]>
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
Signed-off-by: deepthi <[email protected]>
When a query is executed inside a transaction, there is an additional nuance. The actual timeout used will be the smaller | ||
of the transaction timeout and the query timeout. |
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.
@harshit-gangal / @GuptaManan100 is this correct? That is, we pick one value from comment/session var/vtgate flag, and the other value from the transaction timeout, and apply the lower of the two.
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.
Yes -
timeout := tsv.loadQueryTimeoutWithOptions(options)
if txID == 0 {
return timeout
}
// fetch the transaction timeout.
txTimeout := tsv.config.TxTimeoutForWorkload(querypb.ExecuteOptions_OLTP)
// Use the smaller of the two values (0 means infinity).
return smallerTimeout(timeout, txTimeout)
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.
One comment, otherwise it looks good to me
- **[Deprecations and Deletions](#deprecations-and-deletions)** | ||
- [Deprecated VTTablet Flags](#vttablet-flags) | ||
- [Deletion of deprecated metrics](#metric-deletion) | ||
- [VTTablet Flags](#vttablet-flags) | ||
- [Metrics](#deprecations-metrics) | ||
- [Deprecated Metrics](#deprecations-metrics) |
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.
It seems like we are using case, between Deletion of deprecated metrics
and the other items (i.e. Deprecated VTTablet Flags
)
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.
Good point, but I don't think it's very important.
Description
Wordsmith the release summary before code freeze.
Related Issue(s)
Checklist
Deployment Notes