Cherry-pick series of brin-related fixes #912
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We weren't factoring in block sequence limits when we were calculating the starting block of a range. (Fixes #14834) For e.g. if we called brinGetTupleForHeapBlock() with heapBlk = 33554433 and pages_per_range=10, we would calculate the starting block number to be 33554430 (instead of 33554432). Similarly, 33554443, which falls in the next range, would be be resolved to 33554440 (instead of 3355442). So, we now factor in the block sequence's starting block into our calculations. For heap tables, the calculation remains the same as before as the sequence starting block number is always 0. A similar correction has also been applied to brininsert() and brinsummarize().
(1) The test was incorrect. We should have put the fault point before calling table_relation_get_block_sequence() in summarize_range(). Corrected that. (2) This also revealed that we were incorrectly unlocking phbuf in the bail code. (3) There were some diffs from the previous commit which corrected the starting block calculation for ranges. Resolved those. (4) Revamped the tests to cover both scenarios - (a) concurrent insert into the final partial range and (b) concurrent insert to extend the final partial range. (5) Made the placeholder mechanism more prominent in the tests. (6) Also added a test for heap tables to demonstrate (b).
We weren't "processing the final batch" when we were switching block sequences (#15155). This led to us not summarizing the final range in the current block sequence, before switching to the next sequence, during builds. Now, we do so by maintaining a counter of the number of tuples processed for each block sequence, checking that counter and then calling form_and_insert_tuple(), before switching to the next sequence. This is a necessary evil, until we refactor brinbuild to loop over BlockSequences.
This commit adds white-box coverage for cases with aborted rows. The brin_heap test is also modified to serve as a frame of reference.
This commit introduces a fault point in bringetbitmap(), where we add a page into the output bitmap. We add additional coverage for scans now across the existing tests. By actually asserting what gets added in the output tidbitmap, we can not only assert correctness, but we can ensure that extra ranges aren't getting added unnecessarily, to prevent performance pitfalls.
1. brin_revmap_chain() doesn't need a rawpage as arg anymore. 2. Displaying -1 instead of InvalidBlockNumber, thus shortening output.
There is no reason why we need to maintain a counter. Let's use a boolean variable instead. Add appropriate commentary.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #ISSUE_Number
What does this PR do?
Type of Change
Breaking Changes
Test Plan
make installcheckmake -C src/test installcheck-cbdb-parallelImpact
Performance:
User-facing changes:
Dependencies:
Checklist
Additional Context
CI Skip Instructions