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

Optimize the order of compressed chunk metadata columns #6664

Merged
merged 1 commit into from
Feb 27, 2024

Conversation

akuzm
Copy link
Member

@akuzm akuzm commented Feb 16, 2024

Put global metadata columns first, segmentby next, orderby metadata before the compressed orderby columns. This matches the order in which they are likely to be accessed when reading from a compressed chunk.

This affects only the newly created compressed chunks, and the execution over either old or new chunks is not affected, because it does not depend on the particular attribute numbers.

In tsbench, this improves one query which accesses only an early column UserID of a wide table hits. Before this PR, the compressed slot would be deformed entirely, because the count metadata column was after all the compressed columns.
https://grafana.ops.savannah-dev.timescale.com/d/fasYic_4z/compare-akuzm?orgId=1&var-branch=All&var-run1=3273&var-run2=3275&var-threshold=0&var-use_historical_thresholds=true&var-threshold_expression=2.5%20%2A%20percentile_cont%280.90%29&var-exact_suite_version=false

Fixes #5766

Disable-check: force-changelog-file

Copy link

codecov bot commented Feb 19, 2024

Codecov Report

Attention: Patch coverage is 89.18919% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 81.53%. Comparing base (59f50f2) to head (9dfdbc0).
Report is 34 commits behind head on main.

Files Patch % Lines
tsl/src/nodes/decompress_chunk/exec.c 80.00% 0 Missing and 3 partials ⚠️
tsl/src/nodes/decompress_chunk/decompress_chunk.c 66.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6664      +/-   ##
==========================================
+ Coverage   80.06%   81.53%   +1.46%     
==========================================
  Files         190      191       +1     
  Lines       37181    36431     -750     
  Branches     9450     9461      +11     
==========================================
- Hits        29770    29705      -65     
+ Misses       2997     2962      -35     
+ Partials     4414     3764     -650     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@akuzm akuzm marked this pull request as ready for review February 19, 2024 12:21
Copy link
Contributor

@antekresic antekresic left a comment

Choose a reason for hiding this comment

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

🎉

ctid | time | a | b | c | _ts_meta_count | _ts_meta_sequence_num | _ts_meta_min_1 | _ts_meta_max_1
-------+----------------------------------------------------------------------------------+---+---+---+----------------+-----------------------+-------------------------------------+-------------------------------------
(0,2) | BAAAApQ2Uhq14/////5TcU6AAAAAAwAAAAMAAAAAAAAO7gAFKG/+g/vGAAUob/+1KMUAAAADV+w1/w== | 2 | | 2 | 3 | 10 | Sun Jan 01 09:56:20.048355 2023 PST | Sun Jan 01 11:56:20.048355 2023 PST
ctid | _ts_meta_count | _ts_meta_sequence_num | a | c | _ts_meta_min_1 | _ts_meta_max_1 | time | b
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a specific reason for not moving the segment_by columns (i.e., a, c) also to the end and have all _ts_meta columns first?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think filters on segmentby might be more frequent than ordering, so I put segmentby before the orderby metadata. In general for minmax columns, I think might be good to follow the order specified by the user, especially if we're going to introduce minmax for non-orderby columns. But this is just a vague intuition, haven't really done any research on it.

Copy link
Contributor

@jnidzwetzki jnidzwetzki left a comment

Choose a reason for hiding this comment

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

One question regarding the ordering. Rest looks good to me.

@akuzm akuzm merged commit b0b08ab into timescale:main Feb 27, 2024
43 checks passed
@akuzm akuzm deleted the order branch February 27, 2024 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Suboptimal order of columns in compressed chunks
4 participants