-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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 release notes for 0.288 #23079
Add release notes for 0.288 #23079
Conversation
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.
What I did in this review:
- Added PR numbers to each extracted release note entry that was missing them.
- Minor text changes for consistency with the Release Notes Guidelines, such as changing "Upgraded" to "Upgrade".
- Fixed formatting of configuration properties from
this
tothis
. - Added a few links from an entry to appropriate documentation.
WHAT I DID NOT DO:
- Review the Missing Release Notes section to find if any of the entries in Missing Release Notes should have release note entries written and added to this doc update.
- Sort the release note entries in each section to follow the Order of changes in the Release Notes Guidelines. I wanted to focus on fixing the existing individual line items in this review. I or someone else can sort the items in a later review.
I see they are added. |
@tanjialiang Somehow the release note for [[native] Remove spill stats reporting from presto native(https://github.com//pull/22751#top)#22751 was not picked up. But anyways your original release note might be too long. I 'd like to use the following:
Do you think this is correct? |
@tangjiangling Can you please confirm and complete the following release note for [native] Migrate away stats reporting for allocator and cache ? Or you can provide a release note that follows https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines.
|
|
confirmed |
The fix reduces the unnecessary call to metastore API and improves the performance. Yes, the release note looks good to me. |
Co-authored-by: Steve Burnett <[email protected]>
eb800fc
to
e7ca6cc
Compare
Co-authored-by: Ke <[email protected]>
@yingsu00 please add review in the file directly, so that I know the location to update. |
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.
What I did in this review:
- Local doc build of updated branch to check formatting
- Tested and verified all links work
- Checked accuracy of PR # links for each entry, found two links to incorrect PRs / found correct PR / fixed links
- Collapsed the redundant "Hive Changes" and "Iceberg Changes" sections into the "Hive Connector Changes" and "Iceberg Connector Changes" sections
- Sorted the release note entries in each section to follow the Order of changes in the Release Notes Guidelines.
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.
Suppressing warnings generated in local doc build, see #23023 for explanation.
* Improve logging for RowExpressionRewriteRuleSet and StatsRecordingPlanOptimizer optimizers to include more information :pr:`22765`. | ||
* Improve session property ``property-use_broadcast_when_buildsize_small_probeside_unknown`` to do broadcast join when probe side size is unknown and build side estimation from HBO is small. | ||
* Improve the estimation stats recorded during query optimization :pr:`22769 `. | ||
* Add :doc:`/presto_cpp/properties` documentation :pr:`22885`. |
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.
Add the following missing note before this line
Improve join performance by prefiltering the build side with distinct keys from the probe side. This can be enabled with the ``join_prefilter_build_side `` session property. :pr:`22667`
THanks @ClarenceThreepwood . You said "Add framework to add and drop not null column constraints". This "framework" is hard to understand by the users. Can I just say "Add support for
@steveburnett Thanks for the suggestion, however spill_metrics_name is not the actual name. There are 10 memory cache snapshot metrics, 9 memory cache cumulative stats, 3 SSD cache snapshot stats, 21 SSD cache cumulative stats, and 1 TTL controller snapshot stats. I think it's better to list them all so the users know what were moved. What do you think? |
@steveburnett I think this belong to the bigger effort of moving the stats namespace PR in #22751. These stats were not renamed, but just moved to a different namespace. So we can have 1 release note for this starting with "Replace..." |
Thanks for the explanation! I agree. In that case, I'd suggest merely adding the PR number to your phrasing, like so:
|
Add two system configuration properties to specify the reserved query memory capacity on native clusters: |
That looks good, thanks! Shall we add a Presto native keyword here? |
Just talked with @tanjialiang offline, and here's what he suggested:
This may be too long and I have a shorter version
@steveburnett how do you like them? |
@yingsu00, I think your shorter version works great! It's very clear to me what it means. I would suggest adding " :pr: |
@xiaoxmeng explained the two memory counters to me offline yesterday. For that I'm submitting #23126 to try to explain the meaning of these counters better. |
927b86d
to
6e20885
Compare
Just pushed a updated version. Changes are as follows:
|
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.
Two small changes - fix a formatting error, and move an entry to the correct sequence in its group. No content changes to any entry.
Thanks @yingsu00! I pulled the updated branch with your updates and did a new local docs build. Found, fixed, and committed a few small changes, then pulled and built locally again to check that my minor changes didn't break anything new. Also re-checked all PR links (also that each entry has a PR link) and tested all other links to verify that they link to a good destination. I did not find anything else that needs to be addressed at this time. |
Thanks @steveburnett ! The test failure seems irrelevant however I was unable to log into CircleCI to rerun it. @wanglinsong will you be able to rerun the failed tests? |
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.
LGTM! (docs)
Pulled updated branch, new local docs build, looks good.
Do we need to care about failures in the test workflows? |
Yes the tests need to pass. I just restarted the failed test. |
**Credits** | ||
=========== | ||
|
||
8dukongjian, Abhisek Saikia, Ajay Gupte, Amit Dutta, Andrii Rosa, Beinan Wang, Christian Zentgraf, Deepak Majeti, Denodo Research Labs, Elliotte Rusty Harold, Emanuel F, Emanuel F., Fazal Majid, Feilong Liu, Ge Gao, Jalpreet Singh Nanda (:imjalpreet), Jialiang Tan, Jimmy Lu, Jonathan Hehir, Karteekmurthys, Ke, Kevin Wilfong, Konjac Huang, Linsong Wang, Michael Shang, Neerad Somanchi, Nidhin Varghese, Nikhil Collooru, Pranjal Shankhdhar, Rebecca Schlussel, Reetika Agrawal, Rohit Jain, Sean Yeh, Sergey Pershin, Sergii Druzkin, Sreeni Viswanadha, Steve Burnett, Swapnil Tailor, Tishyaa Chaudhry, Vivek, Vivian Hsu, Wills Feng, Yedidya Feldblum, Yihao Zhou, Yihong Wang, Ying Su, Zac Blanco, Zac Wen, abhinavmuk04, aditi-pandit, deepthydavis, jackychen718, jaystarshot, kiersten-stokes, wangd, wypb, xiaoxmeng, ymmarissa |
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.
Emanuel F
appears twice
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.
Emanuel F appears twice
Thanks. Removed.
The TaskManagerTest.buildSpillDirectoryFailure (3352 ms) tests failed. Rerun again to see if it's the same test. |
e8d68c2
to
576645a
Compare
Missing Release Notes
Ajay Gupte
Amit Dutta
Christian Zentgraf
Deepak Majeti
Emanuel F
Jialiang Tan
Jimmy Lu
Karteekmurthys
Ke
Konjac Huang
Linsong Wang
Sreeni Viswanadha
Vivek
Zac Blanco
jackychen718
wypb
xiaoxmeng
Extracted Release Notes
use-new-nan-definition
tofalse
. This configuration property is intended to be temporary to ease migration in the short term, and will be removed in a future release.22417
.22417
.http-server.authentication.allow-forwarded-https
configuration property to recognize X-Forwarded-Proto header, :pr:22492
.node-scheduler.max-preferred-nodes
configuration property to allow changing number of preferred nodes when soft affinity scheduling is enabled. :pr:22562
.hive.affinity-scheduling-file-section-size
configuration property andaffinity_scheduling_file_section_size
session property. The default file size is 256MB. :pr:22563
.22606
.expire_snapshots
to remove old snapshots in Iceberg. :pr:22609
.CAST
"" #22618 (Author: wangd): Revert "Revert "Preserve case for RowType's field name and JSON content whenCAST
""22620
.22652
.22665
.property-use_broadcast_when_buildsize_small_probeside_unknown
to do broadcast join when probe side size is unknown and build side estimation from HBO is small.22700
./functions/noisy
, including :func:noisy_approx_distinct_sfm
and :func:noisy_approx_set_sfm
(:pr:21290
, :pr:22715
).22717
./sql/explain-analyze
statement to support aformat
argument with values of<TEXT|JSON>
:pr:22733
.22737
.22753
.io.jsonwebtoken
artifacts for Java 11 compatibility #22762 (Author: kiersten-stokes): Updateio.jsonwebtoken
artifacts for Java 11 compatibility22762
.cluster-resource-group-state-info-expiration-duration
to a non-zero duration. :pr:22764
.22765
.22769
.noisy_approx_set_sfm_from_index_and_zeros
.22806
.task.max-worker-threads
configuration property to<multiplier>C
. For example, setting the property to2C
configures the worker thread pool to create up to twice as many threads as there are cores available on a machine. :pr:22809
.BEFORE
syntax for Iceberg tables to return historical data :pr:22851
.VERSION (SYSTEM_VERSION)
syntax to include snapshot id using bigint data type :pr:22851
.TIMESTAMP (SYSTEM_TIME)
syntax to include timestamp-with-time-zone data type :pr:22851
.22853
./presto_cpp/properties
documentation :pr:22885
.deprecated.group-by-uses-equal
, which allowed group by to use equal to rather than distinct semantics.NUMERIC_VALUE_OUT_OF_RANGE
toINVALID_CAST_ARGUMENT
.22918
.22926
.ConnectorPageSourceProvider
.All Commits
expire_snapshots
for iceberg (wangd)...iceberg.procedure
(wangd)CAST
"" (wangd)CAST
" (Neerad Somanchi)