Re: Parallel bitmap heap scan

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Dilip Kumar <dilipbalaut(at)gmail(dot)com>
Cc: Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>, Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com>, Rafia Sabih <rafia(dot)sabih(at)enterprisedb(dot)com>, tushar <tushar(dot)ahuja(at)enterprisedb(dot)com>, Amit Khandekar <amitdkhan(dot)pg(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Parallel bitmap heap scan
Date: 2017-03-07 16:11:19
Message-ID: CA+TgmoZdJbzDd15ZAp2sK3+KATyjk+uK74Ln2FcvmLC_1M1FrQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

(On Tue, Feb 28, 2017 at 10:48 AM, Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> 0001- same as previous with some changes for freeing the shared memory stuff.

+ if (--ptbase->refcount == 0)
+ dsa_free(dsa, istate->pagetable);
+
+ if (istate->spages)
+ {
+ ptpages = dsa_get_address(dsa, istate->spages);
+ if (--ptpages->refcount == 0)
+ dsa_free(dsa, istate->spages);
+ }
+ if (istate->schunks)
+ {
+ ptchunks = dsa_get_address(dsa, istate->schunks);
+ if (--ptchunks->refcount == 0)
+ dsa_free(dsa, istate->schunks);
+ }

This doesn't involve any locking, which I think will happen to work
with the current usage pattern but doesn't seem very robust in
general. I think you either need the refcounts to be protected by a
spinlock, or maybe better, use pg_atomic_uint32 for them. You want
something like if (pg_atomic_sub_fetch_u32(&refcount, 1) == 0) {
dsa_free(...) }

Otherwise, there's no guarantee it will get freed exactly once.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2017-03-07 16:14:48 Re: Parallel bitmap heap scan
Previous Message Peter Eisentraut 2017-03-07 16:09:50 Re: PATCH: pageinspect / add page_checksum and bt_page_items(bytea)