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

Fix issues with Hypercore TAM recompression #7364

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

erimatnor
Copy link
Contributor

@erimatnor erimatnor commented Oct 18, 2024

A truncate on a hypercore TAM table is executed across both compressed and non-compressed data. This caused an issue when recompressing because it tries to truncate also the compressed data. Fix this issue by introducing a flag that allows truncating only the non-compressed data.

Another issue releated to cache invalidation is also fixed. Since a recompression sometimes creates a new compressed relation, and the compressed relid is cached in the Hypercore TAM's relcache entry, the cache needs to be invalidated during recompression. However, this wasn't done previously leading to an error. This is fixed by adding a relcache invalidation during recompression.

Finally, compression using an index scan is disabled for Hypercore TAM since the index covers also compressed data (in the recompression case). While the index could be used when compressing the first time (when only non-compressed data is indexed), it is still disabled completely for Hypercore TAM given that index scans are not used by default anyway.

Tests are added to cover all of the issues described above.

Disable-check: force-changelog-file

@erimatnor erimatnor changed the title Refactor option to skip compressed in Hypercore TAM scans Fix issues with Hypercore TAM recompression Oct 18, 2024
Copy link

codecov bot commented Oct 18, 2024

Codecov Report

Attention: Patch coverage is 75.00000% with 6 lines in your changes missing coverage. Please review.

Project coverage is 82.59%. Comparing base (59f50f2) to head (cfbcc74).
Report is 562 commits behind head on main.

Files with missing lines Patch % Lines
tsl/src/hypercore/hypercore_handler.c 54.54% 2 Missing and 3 partials ⚠️
tsl/src/compression/compression.c 83.33% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7364      +/-   ##
==========================================
+ Coverage   80.06%   82.59%   +2.52%     
==========================================
  Files         190      228      +38     
  Lines       37181    42579    +5398     
  Branches     9450    10689    +1239     
==========================================
+ Hits        29770    35167    +5397     
- Misses       2997     3140     +143     
+ Partials     4414     4272     -142     

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

@erimatnor erimatnor marked this pull request as ready for review October 18, 2024 11:51
@erimatnor erimatnor force-pushed the hypercore-fix-recompress branch 3 times, most recently from 61e38f5 to a0bb76d Compare October 22, 2024 08:59
A truncate on a hypercore TAM table is executed across both compressed
and non-compressed data. This caused an issue when recompressing
because it tries to truncate also the compressed data. Fix this issue
by introducing a flag that allows truncating only the non-compressed
data.

Another issue releated to cache invalidation is also fixed. Since a
recompression sometimes creates a new compressed relation, and the
compressed relid is cached in the Hypercore TAM's relcache entry, the
cache needs to be invalidated during recompression. However, this
wasn't done previously leading to an error. This is fixed by adding a
relcache invalidation during recompression.

Finally, compression using an index scan is disabled for Hypercore TAM
since the index covers also compressed data (in the recompression
case). While the index could be used when compressing the first time
(when only non-compressed data is indexed), it is still disabled
completely for Hypercore TAM given that index scans are not used by
default anyway.

Tests are added to cover all of the issues described above.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant