Re: Amcheck verification of GiST and GIN

Kirill Reshke <reshkekirill@gmail.com>

From: Kirill Reshke <reshkekirill@gmail.com>
To: "Andrey M. Borodin" <x4mmm@yandex-team.ru>
Cc: Alexander Lakhin <exclusion@gmail.com>, Andrey Borodin <amborodin86@gmail.com>, Peter Geoghegan <pg@bowt.ie>, Mark Dilger <mark.dilger@enterprisedb.com>, Jose Arthur Benetasso Villanova <jose.arthur@gmail.com>, Andres Freund <andres@anarazel.de>, Nikolay Samokhvalov <samokhvalov@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Tomas Vondra <tomas@vondra.me>
Date: 2024-11-29T17:24:37Z
Lists: pgsql-hackers

Attachments

On Tue, 26 Nov 2024 at 11:50, Kirill Reshke <reshkekirill@gmail.com> wrote:
> === problems with gin_index_check
>
> 1)
> ```
> reshke@ygp-jammy:~/postgres/contrib/amcheck$ ../../pgbin/bin/psql db1
> psql (18devel)
> Type "help" for help.
>
> db1=# select gin_index_check('users_search_idx');
> ERROR:  index "users_search_idx" has wrong tuple order, block 35868, offset 33
> ```
>
> For some reason gin_index_check fails on my index. I am 99% there is
> no corruption in it. Will try to investigate.
>
> 2) this is already discovered by Tomas, but I add my input here:
>
>
> psql session:
> ```
> db1=# set log_min_messages to debug5;
> SET
> db1=# select gin_index_check('users_search_idx');
>
> ```
>
>
> gdb session:
> ```
> (gdb) bt
> #0  __pthread_kill_implementation (no_tid=0, signo=6,
> threadid=140601454760896) at ./nptl/pthread_kill.c:44
> #1  __pthread_kill_internal (signo=6, threadid=140601454760896) at
> ./nptl/pthread_kill.c:78
> #2  __GI___pthread_kill (threadid=140601454760896,
> signo=signo@entry=6) at ./nptl/pthread_kill.c:89
> #3  0x00007fe055af0476 in __GI_raise (sig=sig@entry=6) at
> ../sysdeps/posix/raise.c:26
> #4  0x00007fe055ad67f3 in __GI_abort () at ./stdlib/abort.c:79
> #5  0x000055ea82af4ef0 in ExceptionalCondition
> (conditionName=conditionName@entry=0x7fe04a87aa35
> "ItemPointerIsValid(pointer)",
>     fileName=fileName@entry=0x7fe04a87a928
> "../../src/include/storage/itemptr.h",
> lineNumber=lineNumber@entry=126) at assert.c:66
> #6  0x00007fe04a871372 in ItemPointerGetOffsetNumber
> (pointer=<optimized out>) at ../../src/include/storage/itemptr.h:126
> #7  ItemPointerGetOffsetNumber (pointer=<optimized out>) at
> ../../src/include/storage/itemptr.h:124
> #8  gin_check_posting_tree_parent_keys_consistency
> (posting_tree_root=<optimized out>, rel=<optimized out>) at
> verify_gin.c:296
> #9  gin_check_parent_keys_consistency (rel=rel@entry=0x7fe04a8aa328,
> heaprel=heaprel@entry=0x7fe04a8a9db8,
> callback_state=callback_state@entry=0x0,
> readonly=readonly@entry=false) at verify_gin.c:597
> #10 0x00007fe04a87098d in amcheck_lock_relation_and_check
> (indrelid=16488, am_id=am_id@entry=2742,
> check=check@entry=0x7fe04a870a80 <gin_check_parent_keys_consistency>,
> lockmode=lockmode@entry=1,
>     state=state@entry=0x0) at verify_common.c:132
> #11 0x00007fe04a871e34 in gin_index_check (fcinfo=<optimized out>) at
> verify_gin.c:81
> #12 0x000055ea827cc275 in ExecInterpExpr (state=0x55ea84903390,
> econtext=0x55ea84903138, isnull=<optimized out>) at
> execExprInterp.c:770
> #13 0x000055ea82804fdc in ExecEvalExprSwitchContext
> (isNull=0x7ffeba7fdd37, econtext=0x55ea84903138, state=0x55ea84903390)
> at ../../../src/include/executor/executor.h:367
> #14 ExecProject (projInfo=0x55ea84903388) at
> ../../../src/include/executor/executor.h:401
> #15 ExecResult (pstate=<optimized out>) at nodeResult.c:135
> #16 0x000055ea827d007a in ExecProcNode (node=0x55ea84903028) at
> ../../../src/include/executor/executor.h:278
> #17 ExecutePlan (execute_once=<optimized out>, dest=0x55ea84901940,
> direction=<optimized out>, numberTuples=0, sendTuples=<optimized out>,
> operation=CMD_SELECT, use_parallel_mode=<optimized out>,
>     planstate=0x55ea84903028, estate=0x55ea84902e00) at execMain.c:1655
> #18 standard_ExecutorRun (queryDesc=0x55ea8485c1a0,
> direction=<optimized out>, count=0, execute_once=<optimized out>) at
> execMain.c:362
> #19 0x000055ea829ad6df in PortalRunSelect (portal=0x55ea848b1810,
> forward=<optimized out>, count=0, dest=<optimized out>) at
> pquery.c:924
> #20 0x000055ea829aedc1 in PortalRun
> (portal=portal@entry=0x55ea848b1810,
> count=count@entry=9223372036854775807,
> isTopLevel=isTopLevel@entry=true, run_once=run_once@entry=true,
>     dest=dest@entry=0x55ea84901940,
> altdest=altdest@entry=0x55ea84901940, qc=0x7ffeba7fdfd0) at
> pquery.c:768
> #21 0x000055ea829aab47 in exec_simple_query
> (query_string=0x55ea84831250 "select
> gin_index_check('users_search_idx');") at postgres.c:1283
> #22 0x000055ea829ac777 in PostgresMain (dbname=<optimized out>,
> username=<optimized out>) at postgres.c:4798
> #23 0x000055ea829a6a33 in BackendMain (startup_data=<optimized out>,
> startup_data_len=<optimized out>) at backend_startup.c:107
> #24 0x000055ea8290122f in postmaster_child_launch
> (child_type=<optimized out>, child_slot=1,
> startup_data=startup_data@entry=0x7ffeba7fe48c "",
> startup_data_len=startup_data_len@entry=4,
>     client_sock=client_sock@entry=0x7ffeba7fe490) at launch_backend.c:274
> #25 0x000055ea82904c3f in BackendStartup (client_sock=0x7ffeba7fe490)
> at postmaster.c:3377
> #26 ServerLoop () at postmaster.c:1663
> #27 0x000055ea8290656b in PostmasterMain (argc=argc@entry=3,
> argv=argv@entry=0x55ea8482ab10) at postmaster.c:1361
> #28 0x000055ea825ecc0a in main (argc=3, argv=0x55ea8482ab10) at main.c:196
> (gdb)
> ```
>
> We also need to change the default version of the extension to 1.5.
> I'm not sure which patch of this series should do that.

Both problems resolved with correct maxoff calculation:

```
reshke@ygp-jammy:~/postgres$ git diff contrib/amcheck/verify_gin.c
diff --git a/contrib/amcheck/verify_gin.c b/contrib/amcheck/verify_gin.c
index 39baae40f0c..ddf072d468d 100644
--- a/contrib/amcheck/verify_gin.c
+++ b/contrib/amcheck/verify_gin.c
@@ -412,7 +412,7 @@ gin_check_parent_keys_consistency(Relation rel,
                LockBuffer(buffer, GIN_SHARE);
                page = (Page) BufferGetPage(buffer);
                lsn = BufferGetLSNAtomic(buffer);
-               maxoff = PageGetMaxOffsetNumber(page);
+               maxoff = GinPageGetOpaque(page)->maxoff;

                /* Do basic sanity checks on the page headers */
                check_index_page(rel, buffer, stack->blkno);
```

-- 
Best regards,
Kirill Reshke

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. amcheck: Fix posting tree checks in gin_index_check()

  2. amcheck: Fix parent key check in gin_index_check()

  3. amcheck: Fix checks of entry order for GIN indexes

  4. amcheck: Remove unused GinScanItem->parentlsn field

  5. amcheck: Test gin_index_check on a multicolumn index

  6. Remove incidental md5() function use from test

  7. amcheck: Add a GIN index to the CREATE INDEX CONCURRENTLY tests

  8. amcheck: Add a test with GIN index on JSONB data

  9. amcheck: Fix indentation in verify_gin.c

  10. amcheck: Add gin_index_check() to verify GIN index

  11. amcheck: Move common routines into a separate module

  12. Fix grammar in GIN README

  13. Avoid amcheck inline compression false positives.