pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feed[sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint
14+ messages / 4 participants
[nested] [flat]
* [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint
@ 2017-03-26 08:01 Andreas Seltenreich <seltenreich@gmx.de>
2017-03-27 09:39 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Ashutosh Sharma <ashu.coek88@gmail.com>
0 siblings, 1 reply; 14+ messages in thread
From: Andreas Seltenreich @ 2017-03-26 08:01 UTC (permalink / raw)
To: pgsql-hackers
Hi,
testing with master as of cf366e97ff, sqlsmith occasionally triggers the
following assertion:
TRAP: FailedAssertion("!(LWLockHeldByMe(((LWLock*) (&(bufHdr)->content_lock))))", File: "bufmgr.c", Line: 3397)
Backtraces always look like the one below. It is reproducible on a
cluster once it happens. I could provide a tarball if needed.
regards,
Andreas
#2 0x00000000008324b1 in ExceptionalCondition (conditionName=conditionName@entry=0x9e4e28 "!(LWLockHeldByMe(((LWLock*) (&(bufHdr)->content_lock))))", errorType=errorType@entry=0x87b03d "FailedAssertion", fileName=fileName@entry=0x9e5856 "bufmgr.c", lineNumber=lineNumber@entry=3397) at assert.c:54
#3 0x0000000000706971 in MarkBufferDirtyHint (buffer=2844, buffer_std=buffer_std@entry=1 '\001') at bufmgr.c:3397
#4 0x00000000004b3ecd in _hash_kill_items (scan=scan@entry=0x66dcf70) at hashutil.c:514
#5 0x00000000004a9c1b in hashendscan (scan=0x66dcf70) at hash.c:512
#6 0x00000000004cf17a in index_endscan (scan=0x66dcf70) at indexam.c:353
#7 0x000000000061fa51 in ExecEndIndexScan (node=0x3093f30) at nodeIndexscan.c:852
#8 0x0000000000608e59 in ExecEndNode (node=<optimized out>) at execProcnode.c:715
#9 0x00000000006045b8 in ExecEndPlan (estate=0x3064000, planstate=<optimized out>) at execMain.c:1540
#10 standard_ExecutorEnd (queryDesc=0x30cb880) at execMain.c:487
#11 0x00000000005c87b0 in PortalCleanup (portal=0x1a60060) at portalcmds.c:302
#12 0x000000000085cbb3 in PortalDrop (portal=0x1a60060, isTopCommit=<optimized out>) at portalmem.c:489
#13 0x0000000000736ed2 in exec_simple_query (query_string=0x315b7a0 "...") at postgres.c:1111
#14 0x0000000000738b51 in PostgresMain (argc=<optimized out>, argv=argv@entry=0x1a6c6c8, dbname=<optimized out>, username=<optimized out>) at postgres.c:4071
#15 0x0000000000475fef in BackendRun (port=0x1a65b90) at postmaster.c:4317
#16 BackendStartup (port=0x1a65b90) at postmaster.c:3989
#17 ServerLoop () at postmaster.c:1729
#18 0x00000000006c8662 in PostmasterMain (argc=argc@entry=4, argv=argv@entry=0x1a3f540) at postmaster.c:1337
#19 0x000000000047729d in main (argc=4, argv=0x1a3f540) at main.c:228
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint
2017-03-26 08:01 [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Andreas Seltenreich <seltenreich@gmx.de>
@ 2017-03-27 09:39 ` Ashutosh Sharma <ashu.coek88@gmail.com>
2017-03-27 18:30 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Andreas Seltenreich <seltenreich@gmx.de>
2017-03-31 19:38 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Robert Haas <robertmhaas@gmail.com>
0 siblings, 2 replies; 14+ messages in thread
From: Ashutosh Sharma @ 2017-03-27 09:39 UTC (permalink / raw)
To: Andreas Seltenreich <seltenreich@gmx.de>; +Cc: pgsql-hackers
Hi,
> testing with master as of cf366e97ff, sqlsmith occasionally triggers the
> following assertion:
>
> TRAP: FailedAssertion("!(LWLockHeldByMe(((LWLock*) (&(bufHdr)->content_lock))))", File: "bufmgr.c", Line: 3397)
>
> Backtraces always look like the one below. It is reproducible on a
> cluster once it happens. I could provide a tarball if needed.
>
> regards,
> Andreas
>
> #2 0x00000000008324b1 in ExceptionalCondition (conditionName=conditionName@entry=0x9e4e28 "!(LWLockHeldByMe(((LWLock*) (&(bufHdr)->content_lock))))", errorType=errorType@entry=0x87b03d "FailedAssertion", fileName=fileName@entry=0x9e5856 "bufmgr.c", lineNumber=lineNumber@entry=3397) at assert.c:54
> #3 0x0000000000706971 in MarkBufferDirtyHint (buffer=2844, buffer_std=buffer_std@entry=1 '\001') at bufmgr.c:3397
> #4 0x00000000004b3ecd in _hash_kill_items (scan=scan@entry=0x66dcf70) at hashutil.c:514
> #5 0x00000000004a9c1b in hashendscan (scan=0x66dcf70) at hash.c:512
> #6 0x00000000004cf17a in index_endscan (scan=0x66dcf70) at indexam.c:353
> #7 0x000000000061fa51 in ExecEndIndexScan (node=0x3093f30) at nodeIndexscan.c:852
> #8 0x0000000000608e59 in ExecEndNode (node=<optimized out>) at execProcnode.c:715
> #9 0x00000000006045b8 in ExecEndPlan (estate=0x3064000, planstate=<optimized out>) at execMain.c:1540
> #10 standard_ExecutorEnd (queryDesc=0x30cb880) at execMain.c:487
> #11 0x00000000005c87b0 in PortalCleanup (portal=0x1a60060) at portalcmds.c:302
> #12 0x000000000085cbb3 in PortalDrop (portal=0x1a60060, isTopCommit=<optimized out>) at portalmem.c:489
> #13 0x0000000000736ed2 in exec_simple_query (query_string=0x315b7a0 "...") at postgres.c:1111
> #14 0x0000000000738b51 in PostgresMain (argc=<optimized out>, argv=argv@entry=0x1a6c6c8, dbname=<optimized out>, username=<optimized out>) at postgres.c:4071
> #15 0x0000000000475fef in BackendRun (port=0x1a65b90) at postmaster.c:4317
> #16 BackendStartup (port=0x1a65b90) at postmaster.c:3989
> #17 ServerLoop () at postmaster.c:1729
> #18 0x00000000006c8662 in PostmasterMain (argc=argc@entry=4, argv=argv@entry=0x1a3f540) at postmaster.c:1337
> #19 0x000000000047729d in main (argc=4, argv=0x1a3f540) at main.c:228
>
Hi,
Thanks for reporting this problem. Could you please let me know on for
how long did you run sqlsmith to get this crash. However, I have found
the reason for this crash. This is basically happening when trying to
retrieve the tuples using cursor. Basically the current hash index
scan work tuple-at-a-time which means once it finds tuple on page, it
releases lock from the page but keeps pin on it and finally returns
the tuple. When the requested number of tuples are processed there is
no lock on the page that was being scanned but yes there is a pin on
it. Finally, when trying to close a cursor at the end of scan, if any
killed tuples has been identified we try to first mark these items as
dead with the help of _hash_kill_items(). But, since we only have pin
on this page, the assert check 'LWLockHeldByMe()' fails.
When scanning tuples using normal SELECT * statement, before moving to
next page in a bucket we first deal with all the killed items but we
do this without releasing lock and pin on the current page. Hence,
with SELECT queries this crash is not visible.
The attached patch fixes this. But, please note that all these changes
will get removed with the patch for page scan mode - [1].
[1] - https://www.postgresql.org/message-id/CA%2BTgmobYTvexcjqMhXoNCyEUHChzmdC_2xVGgj7eqaYVgoJA%2Bg%40mail...
--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Attachments:
[application/x-patch] reacquire_lock_hashkillitems_if_required.patch (3.5K, ../../CAE9k0P=V2LhtyeMXd295fhisp=NWUhRVJ9EZQCDowWiY9rSohQ@mail.gmail.com/2-reacquire_lock_hashkillitems_if_required.patch)
download | inline diff:
diff --git a/src/backend/access/hash/hash.c b/src/backend/access/hash/hash.c
index 34cc08f..0bacef8 100644
--- a/src/backend/access/hash/hash.c
+++ b/src/backend/access/hash/hash.c
@@ -478,7 +478,7 @@ hashrescan(IndexScanDesc scan, ScanKey scankey, int nscankeys,
/* Before leaving current page, deal with any killed items */
if (so->numKilled > 0)
- _hash_kill_items(scan);
+ _hash_kill_items(scan, false);
_hash_dropscanbuf(rel, so);
@@ -509,7 +509,7 @@ hashendscan(IndexScanDesc scan)
/* Before leaving current page, deal with any killed items */
if (so->numKilled > 0)
- _hash_kill_items(scan);
+ _hash_kill_items(scan, false);
_hash_dropscanbuf(rel, so);
diff --git a/src/backend/access/hash/hashsearch.c b/src/backend/access/hash/hashsearch.c
index 2d92049..414cc6a 100644
--- a/src/backend/access/hash/hashsearch.c
+++ b/src/backend/access/hash/hashsearch.c
@@ -467,7 +467,7 @@ _hash_step(IndexScanDesc scan, Buffer *bufP, ScanDirection dir)
/* Before leaving current page, deal with any killed items */
if (so->numKilled > 0)
- _hash_kill_items(scan);
+ _hash_kill_items(scan, true);
/*
* ran off the end of this page, try the next
@@ -524,7 +524,7 @@ _hash_step(IndexScanDesc scan, Buffer *bufP, ScanDirection dir)
/* Before leaving current page, deal with any killed items */
if (so->numKilled > 0)
- _hash_kill_items(scan);
+ _hash_kill_items(scan, true);
/*
* ran off the end of this page, try the next
diff --git a/src/backend/access/hash/hashutil.c b/src/backend/access/hash/hashutil.c
index 2e99719..f24bc4c 100644
--- a/src/backend/access/hash/hashutil.c
+++ b/src/backend/access/hash/hashutil.c
@@ -456,11 +456,15 @@ _hash_get_newbucket_from_oldbucket(Relation rel, Bucket old_bucket,
* current page and killed tuples thereon (generally, this should only be
* called if so->numKilled > 0).
*
+ * The caller must have pin on so->hashso_curbuf, but may or may not have
+ * read-lock, as indicated by haveLock. Note that we assume read-lock
+ * is sufficient for setting LP_DEAD hint bits.
+ *
* We match items by heap TID before assuming they are the right ones to
* delete.
*/
void
-_hash_kill_items(IndexScanDesc scan)
+_hash_kill_items(IndexScanDesc scan, bool haveLock)
{
HashScanOpaque so = (HashScanOpaque) scan->opaque;
Page page;
@@ -472,6 +476,10 @@ _hash_kill_items(IndexScanDesc scan)
Assert(so->numKilled > 0);
Assert(so->killedItems != NULL);
+ Assert(BufferIsValid(so->hashso_curbuf));
+
+ if (!haveLock)
+ LockBuffer(so->hashso_curbuf, BUFFER_LOCK_SHARE);
/*
* Always reset the scan state, so we don't look for same
@@ -513,4 +521,7 @@ _hash_kill_items(IndexScanDesc scan)
opaque->hasho_flag |= LH_PAGE_HAS_DEAD_TUPLES;
MarkBufferDirtyHint(so->hashso_curbuf, true);
}
+
+ if (!haveLock)
+ LockBuffer(so->hashso_curbuf, BUFFER_LOCK_UNLOCK);
}
diff --git a/src/include/access/hash.h b/src/include/access/hash.h
index eb1df57..89fc319 100644
--- a/src/include/access/hash.h
+++ b/src/include/access/hash.h
@@ -393,7 +393,7 @@ extern BlockNumber _hash_get_oldblock_from_newbucket(Relation rel, Bucket new_bu
extern BlockNumber _hash_get_newblock_from_oldbucket(Relation rel, Bucket old_bucket);
extern Bucket _hash_get_newbucket_from_oldbucket(Relation rel, Bucket old_bucket,
uint32 lowmask, uint32 maxbucket);
-extern void _hash_kill_items(IndexScanDesc scan);
+extern void _hash_kill_items(IndexScanDesc scan, bool haveLock);
/* hash.c */
extern void hashbucketcleanup(Relation rel, Bucket cur_bucket,
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint
2017-03-26 08:01 [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Andreas Seltenreich <seltenreich@gmx.de>
2017-03-27 09:39 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Ashutosh Sharma <ashu.coek88@gmail.com>
@ 2017-03-27 18:30 ` Andreas Seltenreich <seltenreich@gmx.de>
2017-03-28 01:14 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Ashutosh Sharma <ashu.coek88@gmail.com>
2017-04-01 10:45 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Andreas Seltenreich <seltenreich@gmx.de>
1 sibling, 2 replies; 14+ messages in thread
From: Andreas Seltenreich @ 2017-03-27 18:30 UTC (permalink / raw)
To: Ashutosh Sharma <ashu.coek88@gmail.com>; +Cc: pgsql-hackers
Ashutosh Sharma writes:
>> TRAP: FailedAssertion("!(LWLockHeldByMe(((LWLock*) (&(bufHdr)->content_lock))))", File: "bufmgr.c", Line: 3397)
> Thanks for reporting this problem. Could you please let me know on for
> how long did you run sqlsmith to get this crash.
I got about one TRAP per hour when testing on 20 nodes with one postgres
and 5 sqlsmithes on each. Nodes are tiny consumer machines with
low-power 4-core sandy bridges.
> [2. reacquire_lock_hashkillitems_if_required.patch]
I'll test with your patch applied as soon as time permits and report
back.
regards,
Andreas
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint
2017-03-26 08:01 [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Andreas Seltenreich <seltenreich@gmx.de>
2017-03-27 09:39 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Ashutosh Sharma <ashu.coek88@gmail.com>
2017-03-27 18:30 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Andreas Seltenreich <seltenreich@gmx.de>
@ 2017-03-28 01:14 ` Ashutosh Sharma <ashu.coek88@gmail.com>
1 sibling, 0 replies; 14+ messages in thread
From: Ashutosh Sharma @ 2017-03-28 01:14 UTC (permalink / raw)
To: Andreas Seltenreich <seltenreich@gmx.de>; +Cc: pgsql-hackers
On Mar 28, 2017 00:00, "Andreas Seltenreich" <seltenreich@gmx.de> wrote:
Ashutosh Sharma writes:
>> TRAP: FailedAssertion("!(LWLockHeldByMe(((LWLock*)
(&(bufHdr)->content_lock))))", File: "bufmgr.c", Line: 3397)
> Thanks for reporting this problem. Could you please let me know on for
> how long did you run sqlsmith to get this crash.
I got about one TRAP per hour when testing on 20 nodes with one postgres
and 5 sqlsmithes on each. Nodes are tiny consumer machines with
low-power 4-core sandy bridges.
Okay. Thanks for sharing this information. I tried running running sqlsmith
on a single node. I ran it for an hour but didn't find any crash. Let me
also try running multiple sqlsmith at a time...May be 5 like you are doing
on a single node.
> [2. reacquire_lock_hashkillitems_if_required.patch]
I'll test with your patch applied as soon as time permits and report
back.
Sure, Thanks a lot.
With Regards,
Ashutosh Sharma
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint
2017-03-26 08:01 [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Andreas Seltenreich <seltenreich@gmx.de>
2017-03-27 09:39 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Ashutosh Sharma <ashu.coek88@gmail.com>
2017-03-27 18:30 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Andreas Seltenreich <seltenreich@gmx.de>
@ 2017-04-01 10:45 ` Andreas Seltenreich <seltenreich@gmx.de>
2017-04-01 10:54 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Ashutosh Sharma <ashu.coek88@gmail.com>
1 sibling, 1 reply; 14+ messages in thread
From: Andreas Seltenreich @ 2017-04-01 10:45 UTC (permalink / raw)
To: Ashutosh Sharma <ashu.coek88@gmail.com>; +Cc: pgsql-hackers
Andreas Seltenreich writes:
>>> TRAP: FailedAssertion("!(LWLockHeldByMe(((LWLock*) (&(bufHdr)->content_lock))))", File: "bufmgr.c", Line: 3397)
> I got about one TRAP per hour when testing on 20 nodes with one postgres
> and 5 sqlsmithes on each.
> Ashutosh Sharma writes:
>> [2. reacquire_lock_hashkillitems_if_required.patch]
I've done about 12 hours of testing since applying this patch and no
failed assertions so far.
regards,
Andreas
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint
2017-03-26 08:01 [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Andreas Seltenreich <seltenreich@gmx.de>
2017-03-27 09:39 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Ashutosh Sharma <ashu.coek88@gmail.com>
2017-03-27 18:30 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Andreas Seltenreich <seltenreich@gmx.de>
2017-04-01 10:45 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Andreas Seltenreich <seltenreich@gmx.de>
@ 2017-04-01 10:54 ` Ashutosh Sharma <ashu.coek88@gmail.com>
0 siblings, 0 replies; 14+ messages in thread
From: Ashutosh Sharma @ 2017-04-01 10:54 UTC (permalink / raw)
To: Andreas Seltenreich <seltenreich@gmx.de>; +Cc: pgsql-hackers
Hi Andreas,
On Apr 1, 2017 16:15, "Andreas Seltenreich" <seltenreich@gmx.de> wrote:
Andreas Seltenreich writes:
>>> TRAP: FailedAssertion("!(LWLockHeldByMe(((LWLock*)
(&(bufHdr)->content_lock))))", File: "bufmgr.c", Line: 3397)
> I got about one TRAP per hour when testing on 20 nodes with one postgres
> and 5 sqlsmithes on each.
> Ashutosh Sharma writes:
>> [2. reacquire_lock_hashkillitems_if_required.patch]
I've done about 12 hours of testing since applying this patch and no
failed assertions so far.
Thanks for testing my patch. Just FYI, I have slightly changed my patch
this morning as per Robert's suggestions. But, still more or less the logic
remains the same. Thank you once again.
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint
2017-03-26 08:01 [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Andreas Seltenreich <seltenreich@gmx.de>
2017-03-27 09:39 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Ashutosh Sharma <ashu.coek88@gmail.com>
@ 2017-03-31 19:38 ` Robert Haas <robertmhaas@gmail.com>
2017-03-31 22:09 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Ashutosh Sharma <ashu.coek88@gmail.com>
1 sibling, 1 reply; 14+ messages in thread
From: Robert Haas @ 2017-03-31 19:38 UTC (permalink / raw)
To: Ashutosh Sharma <ashu.coek88@gmail.com>; +Cc: Andreas Seltenreich <seltenreich@gmx.de>; pgsql-hackers
On Mon, Mar 27, 2017 at 5:39 AM, Ashutosh Sharma <ashu.coek88@gmail.com> wrote:
> Thanks for reporting this problem. Could you please let me know on for
> how long did you run sqlsmith to get this crash. However, I have found
> the reason for this crash. This is basically happening when trying to
> retrieve the tuples using cursor. Basically the current hash index
> scan work tuple-at-a-time which means once it finds tuple on page, it
> releases lock from the page but keeps pin on it and finally returns
> the tuple. When the requested number of tuples are processed there is
> no lock on the page that was being scanned but yes there is a pin on
> it. Finally, when trying to close a cursor at the end of scan, if any
> killed tuples has been identified we try to first mark these items as
> dead with the help of _hash_kill_items(). But, since we only have pin
> on this page, the assert check 'LWLockHeldByMe()' fails.
Instead of adding bool haveLock to _hash_kill_items, how about just
having the caller acquire the lock before calling the function and
release it afterwards? Since the acquire is at the beginning of the
function and the release at the end, there seems to be no point in
putting the acquire/release inside the function rather than in the
caller.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint
2017-03-26 08:01 [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Andreas Seltenreich <seltenreich@gmx.de>
2017-03-27 09:39 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Ashutosh Sharma <ashu.coek88@gmail.com>
2017-03-31 19:38 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Robert Haas <robertmhaas@gmail.com>
@ 2017-03-31 22:09 ` Ashutosh Sharma <ashu.coek88@gmail.com>
2017-04-01 00:30 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Robert Haas <robertmhaas@gmail.com>
0 siblings, 1 reply; 14+ messages in thread
From: Ashutosh Sharma @ 2017-03-31 22:09 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: Andreas Seltenreich <seltenreich@gmx.de>; pgsql-hackers
On Sat, Apr 1, 2017 at 1:08 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Mon, Mar 27, 2017 at 5:39 AM, Ashutosh Sharma <ashu.coek88@gmail.com> wrote:
>> Thanks for reporting this problem. Could you please let me know on for
>> how long did you run sqlsmith to get this crash. However, I have found
>> the reason for this crash. This is basically happening when trying to
>> retrieve the tuples using cursor. Basically the current hash index
>> scan work tuple-at-a-time which means once it finds tuple on page, it
>> releases lock from the page but keeps pin on it and finally returns
>> the tuple. When the requested number of tuples are processed there is
>> no lock on the page that was being scanned but yes there is a pin on
>> it. Finally, when trying to close a cursor at the end of scan, if any
>> killed tuples has been identified we try to first mark these items as
>> dead with the help of _hash_kill_items(). But, since we only have pin
>> on this page, the assert check 'LWLockHeldByMe()' fails.
>
> Instead of adding bool haveLock to _hash_kill_items, how about just
> having the caller acquire the lock before calling the function and
> release it afterwards? Since the acquire is at the beginning of the
> function and the release at the end, there seems to be no point in
> putting the acquire/release inside the function rather than in the
> caller.
Well, That is another option but the main idea was to be inline with
the btree code. Moreover, I am not sure if acquiring lwlock inside
hashendscan (basically the place where we are trying to close down the
things) would look good.
--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint
2017-03-26 08:01 [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Andreas Seltenreich <seltenreich@gmx.de>
2017-03-27 09:39 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Ashutosh Sharma <ashu.coek88@gmail.com>
2017-03-31 19:38 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Robert Haas <robertmhaas@gmail.com>
2017-03-31 22:09 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Ashutosh Sharma <ashu.coek88@gmail.com>
@ 2017-04-01 00:30 ` Robert Haas <robertmhaas@gmail.com>
2017-04-01 01:36 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Ashutosh Sharma <ashu.coek88@gmail.com>
2017-04-01 12:41 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Amit Kapila <amit.kapila16@gmail.com>
0 siblings, 2 replies; 14+ messages in thread
From: Robert Haas @ 2017-04-01 00:30 UTC (permalink / raw)
To: Ashutosh Sharma <ashu.coek88@gmail.com>; +Cc: Andreas Seltenreich <seltenreich@gmx.de>; pgsql-hackers
On Fri, Mar 31, 2017 at 6:09 PM, Ashutosh Sharma <ashu.coek88@gmail.com> wrote:
> Well, That is another option but the main idea was to be inline with
> the btree code.
That's not a bad goal in principal, but _bt_killitems() doesn't have
any similar argument.
(Also, speaking of consistency, why did we end up with
_hash_kill_items, with an underscore between kill and items, and
_bt_killitems, without one?)
> Moreover, I am not sure if acquiring lwlock inside
> hashendscan (basically the place where we are trying to close down the
> things) would look good.
Well, if that's not a good thing to do, hiding it inside some other
function doesn't make it better. I think it's fine, though.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint
2017-03-26 08:01 [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Andreas Seltenreich <seltenreich@gmx.de>
2017-03-27 09:39 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Ashutosh Sharma <ashu.coek88@gmail.com>
2017-03-31 19:38 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Robert Haas <robertmhaas@gmail.com>
2017-03-31 22:09 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Ashutosh Sharma <ashu.coek88@gmail.com>
2017-04-01 00:30 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Robert Haas <robertmhaas@gmail.com>
@ 2017-04-01 01:36 ` Ashutosh Sharma <ashu.coek88@gmail.com>
2017-04-01 02:02 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Ashutosh Sharma <ashu.coek88@gmail.com>
1 sibling, 1 reply; 14+ messages in thread
From: Ashutosh Sharma @ 2017-04-01 01:36 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: Andreas Seltenreich <seltenreich@gmx.de>; pgsql-hackers
On Sat, Apr 1, 2017 at 6:00 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Fri, Mar 31, 2017 at 6:09 PM, Ashutosh Sharma <ashu.coek88@gmail.com> wrote:
>> Well, That is another option but the main idea was to be inline with
>> the btree code.
>
> That's not a bad goal in principal, but _bt_killitems() doesn't have
> any similar argument.
It was there but later got removed with some patch (may be the patch
for reducing pinning and buffer content lock for btree scans). The
following commit has this changes.
commit 09cb5c0e7d6fbc9dee26dc429e4fc0f2a88e5272
Author: Tom Lane <tgl@sss.pgh.pa.us>
Date: Sun May 7 01:21:30 2006 +0000
>
> (Also, speaking of consistency, why did we end up with
> _hash_kill_items, with an underscore between kill and items, and
> _bt_killitems, without one?)
That is just to follow the naming convention in hash.h Please check
the review comments for this at - [1].
>
>> Moreover, I am not sure if acquiring lwlock inside
>> hashendscan (basically the place where we are trying to close down the
>> things) would look good.
>
> Well, if that's not a good thing to do, hiding it inside some other
> function doesn't make it better.
okay, agreed. I will submit the patch very shortly.
[1] - https://www.postgresql.org/message-id/cf6abd8a-77b5-f1c7-8e50-5bef461e0522%40redhat.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint
2017-03-26 08:01 [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Andreas Seltenreich <seltenreich@gmx.de>
2017-03-27 09:39 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Ashutosh Sharma <ashu.coek88@gmail.com>
2017-03-31 19:38 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Robert Haas <robertmhaas@gmail.com>
2017-03-31 22:09 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Ashutosh Sharma <ashu.coek88@gmail.com>
2017-04-01 00:30 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Robert Haas <robertmhaas@gmail.com>
2017-04-01 01:36 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Ashutosh Sharma <ashu.coek88@gmail.com>
@ 2017-04-01 02:02 ` Ashutosh Sharma <ashu.coek88@gmail.com>
2017-04-04 02:29 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Robert Haas <robertmhaas@gmail.com>
0 siblings, 1 reply; 14+ messages in thread
From: Ashutosh Sharma @ 2017-04-01 02:02 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: Andreas Seltenreich <seltenreich@gmx.de>; pgsql-hackers
Hi,
On Sat, Apr 1, 2017 at 7:06 AM, Ashutosh Sharma <ashu.coek88@gmail.com> wrote:
> On Sat, Apr 1, 2017 at 6:00 AM, Robert Haas <robertmhaas@gmail.com> wrote:
>> On Fri, Mar 31, 2017 at 6:09 PM, Ashutosh Sharma <ashu.coek88@gmail.com> wrote:
>>> Well, That is another option but the main idea was to be inline with
>>> the btree code.
>>
>> That's not a bad goal in principal, but _bt_killitems() doesn't have
>> any similar argument.
>
> It was there but later got removed with some patch (may be the patch
> for reducing pinning and buffer content lock for btree scans). The
> following commit has this changes.
>
> commit 09cb5c0e7d6fbc9dee26dc429e4fc0f2a88e5272
> Author: Tom Lane <tgl@sss.pgh.pa.us>
> Date: Sun May 7 01:21:30 2006 +0000
>
>>
>> (Also, speaking of consistency, why did we end up with
>> _hash_kill_items, with an underscore between kill and items, and
>> _bt_killitems, without one?)
>
> That is just to follow the naming convention in hash.h Please check
> the review comments for this at - [1].
>
>>
>>> Moreover, I am not sure if acquiring lwlock inside
>>> hashendscan (basically the place where we are trying to close down the
>>> things) would look good.
>>
>> Well, if that's not a good thing to do, hiding it inside some other
>> function doesn't make it better.
>
> okay, agreed. I will submit the patch very shortly.
As suggested, I am now acquiring lock inside the caller function.
Attached is the patch having this changes. Thanks.
>
>
> [1] - https://www.postgresql.org/message-id/cf6abd8a-77b5-f1c7-8e50-5bef461e0522%40redhat.com
--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Attachments:
[text/x-patch] acquire_lock_when_calling_hash_kill_items.patch (1.3K, ../../CAE9k0P=3rtgUtxopG+XQVxgASFzAnGd9sNmYUaj_=KeVsKGUdA@mail.gmail.com/2-acquire_lock_when_calling_hash_kill_items.patch)
download | inline diff:
diff --git a/src/backend/access/hash/hash.c b/src/backend/access/hash/hash.c
index 34cc08f..b835f77 100644
--- a/src/backend/access/hash/hash.c
+++ b/src/backend/access/hash/hash.c
@@ -476,9 +476,17 @@ hashrescan(IndexScanDesc scan, ScanKey scankey, int nscankeys,
HashScanOpaque so = (HashScanOpaque) scan->opaque;
Relation rel = scan->indexRelation;
- /* Before leaving current page, deal with any killed items */
+ /*
+ * Before leaving current page, deal with any killed items.
+ * Also, ensure that we acquire lock on current page before
+ * calling _hash_kill_items.
+ */
if (so->numKilled > 0)
+ {
+ LockBuffer(so->hashso_curbuf, BUFFER_LOCK_SHARE);
_hash_kill_items(scan);
+ LockBuffer(so->hashso_curbuf, BUFFER_LOCK_UNLOCK);
+ }
_hash_dropscanbuf(rel, so);
@@ -507,9 +515,17 @@ hashendscan(IndexScanDesc scan)
HashScanOpaque so = (HashScanOpaque) scan->opaque;
Relation rel = scan->indexRelation;
- /* Before leaving current page, deal with any killed items */
+ /*
+ * Before leaving current page, deal with any killed items.
+ * Also, ensure that we acquire lock on current page before
+ * calling _hash_kill_items.
+ */
if (so->numKilled > 0)
+ {
+ LockBuffer(so->hashso_curbuf, BUFFER_LOCK_SHARE);
_hash_kill_items(scan);
+ LockBuffer(so->hashso_curbuf, BUFFER_LOCK_UNLOCK);
+ }
_hash_dropscanbuf(rel, so);
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint
2017-03-26 08:01 [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Andreas Seltenreich <seltenreich@gmx.de>
2017-03-27 09:39 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Ashutosh Sharma <ashu.coek88@gmail.com>
2017-03-31 19:38 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Robert Haas <robertmhaas@gmail.com>
2017-03-31 22:09 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Ashutosh Sharma <ashu.coek88@gmail.com>
2017-04-01 00:30 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Robert Haas <robertmhaas@gmail.com>
2017-04-01 01:36 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Ashutosh Sharma <ashu.coek88@gmail.com>
2017-04-01 02:02 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Ashutosh Sharma <ashu.coek88@gmail.com>
@ 2017-04-04 02:29 ` Robert Haas <robertmhaas@gmail.com>
0 siblings, 0 replies; 14+ messages in thread
From: Robert Haas @ 2017-04-04 02:29 UTC (permalink / raw)
To: Ashutosh Sharma <ashu.coek88@gmail.com>; +Cc: Andreas Seltenreich <seltenreich@gmx.de>; pgsql-hackers
On Fri, Mar 31, 2017 at 10:02 PM, Ashutosh Sharma <ashu.coek88@gmail.com> wrote:
> As suggested, I am now acquiring lock inside the caller function.
> Attached is the patch having this changes. Thanks.
Committed.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint
2017-03-26 08:01 [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Andreas Seltenreich <seltenreich@gmx.de>
2017-03-27 09:39 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Ashutosh Sharma <ashu.coek88@gmail.com>
2017-03-31 19:38 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Robert Haas <robertmhaas@gmail.com>
2017-03-31 22:09 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Ashutosh Sharma <ashu.coek88@gmail.com>
2017-04-01 00:30 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Robert Haas <robertmhaas@gmail.com>
@ 2017-04-01 12:41 ` Amit Kapila <amit.kapila16@gmail.com>
2017-04-01 12:47 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Ashutosh Sharma <ashu.coek88@gmail.com>
1 sibling, 1 reply; 14+ messages in thread
From: Amit Kapila @ 2017-04-01 12:41 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: Ashutosh Sharma <ashu.coek88@gmail.com>; Andreas Seltenreich <seltenreich@gmx.de>; pgsql-hackers
On Sat, Apr 1, 2017 at 6:00 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Fri, Mar 31, 2017 at 6:09 PM, Ashutosh Sharma <ashu.coek88@gmail.com> wrote:
>> Well, That is another option but the main idea was to be inline with
>> the btree code.
>
> That's not a bad goal in principal, but _bt_killitems() doesn't have
> any similar argument.
>
> (Also, speaking of consistency, why did we end up with
> _hash_kill_items, with an underscore between kill and items, and
> _bt_killitems, without one?)
>
>> Moreover, I am not sure if acquiring lwlock inside
>> hashendscan (basically the place where we are trying to close down the
>> things) would look good.
>
> Well, if that's not a good thing to do, hiding it inside some other
> function doesn't make it better. I think it's fine, though.
>
One thing to note here is that this fix won't be required if we get
the page-scan-mode patch [1] in this CF. I think if we fix this with
the patch proposed by Ashutosh, then we anyway needs to again change
the related code (kind of revert the fix) after page-scan-mode patch.
Now, if you think we have negligible chance of getting that patch,
then it is good to proceed with this fix.
[1] - https://commitfest.postgresql.org/13/999/
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
^ permalink raw reply [nested|flat] 14+ messages in thread
* Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint
2017-03-26 08:01 [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Andreas Seltenreich <seltenreich@gmx.de>
2017-03-27 09:39 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Ashutosh Sharma <ashu.coek88@gmail.com>
2017-03-31 19:38 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Robert Haas <robertmhaas@gmail.com>
2017-03-31 22:09 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Ashutosh Sharma <ashu.coek88@gmail.com>
2017-04-01 00:30 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Robert Haas <robertmhaas@gmail.com>
2017-04-01 12:41 ` Re: [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Amit Kapila <amit.kapila16@gmail.com>
@ 2017-04-01 12:47 ` Ashutosh Sharma <ashu.coek88@gmail.com>
0 siblings, 0 replies; 14+ messages in thread
From: Ashutosh Sharma @ 2017-04-01 12:47 UTC (permalink / raw)
To: Amit Kapila <amit.kapila16@gmail.com>; +Cc: pgsql-hackers; Andreas Seltenreich <seltenreich@gmx.de>; Robert Haas <robertmhaas@gmail.com>
On Apr 1, 2017 18:11, "Amit Kapila" <amit.kapila16@gmail.com> wrote:
On Sat, Apr 1, 2017 at 6:00 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Fri, Mar 31, 2017 at 6:09 PM, Ashutosh Sharma <ashu.coek88@gmail.com>
wrote:
>> Well, That is another option but the main idea was to be inline with
>> the btree code.
>
> That's not a bad goal in principal, but _bt_killitems() doesn't have
> any similar argument.
>
> (Also, speaking of consistency, why did we end up with
> _hash_kill_items, with an underscore between kill and items, and
> _bt_killitems, without one?)
>
>> Moreover, I am not sure if acquiring lwlock inside
>> hashendscan (basically the place where we are trying to close down the
>> things) would look good.
>
> Well, if that's not a good thing to do, hiding it inside some other
> function doesn't make it better. I think it's fine, though.
>
One thing to note here is that this fix won't be required if we get
the page-scan-mode patch [1] in this CF. I think if we fix this with
the patch proposed by Ashutosh, then we anyway needs to again change
the related code (kind of revert the fix) after page-scan-mode patch.
Now, if you think we have negligible chance of getting that patch,
then it is good to proceed with this fix.
Yes, I had already mentioned this point in my very first mail. Thanks for
highlighting this once again.
[1] - https://commitfest.postgresql.org/13/999/
^ permalink raw reply [nested|flat] 14+ messages in thread
end of thread, other threads:[~2017-04-04 02:29 UTC | newest]
Thread overview: 14+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2017-03-26 08:01 [sqlsmith] Failed assertion in _hash_kill_items/MarkBufferDirtyHint Andreas Seltenreich <seltenreich@gmx.de>
2017-03-27 09:39 ` Ashutosh Sharma <ashu.coek88@gmail.com>
2017-03-27 18:30 ` Andreas Seltenreich <seltenreich@gmx.de>
2017-03-28 01:14 ` Ashutosh Sharma <ashu.coek88@gmail.com>
2017-04-01 10:45 ` Andreas Seltenreich <seltenreich@gmx.de>
2017-04-01 10:54 ` Ashutosh Sharma <ashu.coek88@gmail.com>
2017-03-31 19:38 ` Robert Haas <robertmhaas@gmail.com>
2017-03-31 22:09 ` Ashutosh Sharma <ashu.coek88@gmail.com>
2017-04-01 00:30 ` Robert Haas <robertmhaas@gmail.com>
2017-04-01 01:36 ` Ashutosh Sharma <ashu.coek88@gmail.com>
2017-04-01 02:02 ` Ashutosh Sharma <ashu.coek88@gmail.com>
2017-04-04 02:29 ` Robert Haas <robertmhaas@gmail.com>
2017-04-01 12:41 ` Amit Kapila <amit.kapila16@gmail.com>
2017-04-01 12:47 ` Ashutosh Sharma <ashu.coek88@gmail.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox