| From: | Andrey Borodin <x4mmm(at)yandex-team(dot)ru> |
|---|---|
| To: | Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com> |
| Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: amcheck: add index-all-keys-match verification for B-Tree |
| Date: | 2026-03-05 06:00:24 |
| Message-ID: | 35809064-01A3-4449-B393-0F71D4C18F7D@yandex-team.ru |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> On 3 Mar 2026, at 04:19, Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com> wrote:
>
> I don't see any other logic problems in the code, I only have a few
> minor comments/questions:
>
> + * Bloom filter says (key, tid) not in heap. Follow TID to verify; this
> + * amortizes random heap lookups when the filter has false negatives, or
>
> This comment could be a bit confusing, as bloom filters typically have
> false positives, not negatives.
> Maybe it would be better to phrase this somehow differently?
>
> Another thing is that now amcheck can create two bloom filters,
> allocated at the same time, both up to maintenance_work_mem.
> Isn't that a detail that should be at least mentioned somewhere?
>
> And in the documentation, shouldn't this new check mention something
> similar to heapallindexed, which describes the check possibly missing
> corruption because of the bloom filter?
>
> + (errcode(ERRCODE_INDEX_CORRUPTED),
> + errmsg("index tuple in index \"%s\" does not match heap tuple",
> + RelationGetRelationName(state->rel)),
>
> Shouldn't this also print out the table name?
Thanks for the review! I've addressed all these issues in patch steps 4 and 5. Other steps are intact. PFA.
Best regards, Andrey Borodin.
| Attachment | Content-Type | Size |
|---|---|---|
| v4-0001-amcheck-add-indexallkeysmatch-verification-for-B-.patch | application/octet-stream | 28.1 KB |
| v4-0005-amcheck-docs-note-two-Bloom-filters-memory-usage-.patch | application/octet-stream | 1.7 KB |
| v4-0004-amcheck-address-review-fix-Bloom-filter-comment-a.patch | application/octet-stream | 2.8 KB |
| v4-0003-Document-indexallkeysmatch-parameter-for-amcheck-.patch | application/octet-stream | 5.8 KB |
| v4-0002-amcheck-report-corruption-when-index-points-to-no.patch | application/octet-stream | 2.7 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Joel Jacobson | 2026-03-05 06:17:29 | Re: [BUG?] estimate_hash_bucket_stats uses wrong ndistinct for avgfreq |
| Previous Message | Chao Li | 2026-03-05 05:58:12 | Re: [BUG + PATCH] DSA pagemap out-of-bounds in make_new_segment odd-sized path |