On 20/01/2016 15:17, Fujii Masao wrote:
> When I compiled the PostgreSQL with the patch, I got the following error.
> ISTM that the inclusion of pg_am.h header file is missing in ginfast.c.
> ginfast.c: In function 'gin_clean_pending_list':
> ginfast.c:980: error: 'GIN_AM_OID' undeclared (first use in this function)
> ginfast.c:980: error: (Each undeclared identifier is reported only once
> ginfast.c:980: error: for each function it appears in.)
> gin_clean_pending_list() must check whether the server is in recovery or not.
> If it's in recovery, the function must exit with an error. That is, IMO,
> something like the following check must be added.
>          if (RecoveryInProgress())
>                  ereport(ERROR,
>                                   errmsg("recovery is in progress"),
>                                   errhint("GIN pending list cannot be
> cleaned up during recovery.")));
> It's better to make gin_clean_pending_list() check whether the target index
> is temporary index of other sessions or not, like pgstatginindex() does.
> +    Relation    indexRel = index_open(indexoid, AccessShareLock);
> ISTM that AccessShareLock is not safe when updating the pending list and
> GIN index main structure. Probaby we should use RowExclusiveLock?

This lock should be safe, as pointed by Alvaro and Jaime earlier in this
as ginInsertCleanup() can be called concurrently.

Also, since 38710a374ea the ginInsertCleanup() call must be fixed:

-   ginInsertCleanup(&ginstate, false, true, &stats);
+   ginInsertCleanup(&ginstate, true, &stats);


Julien Rouhaud
http://dalibo.com - http://dalibo.org

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to