Issue 7829 - MDB mdb_cursor_del causes records to be skipped
Summary: MDB mdb_cursor_del causes records to be skipped
Status: VERIFIED FIXED
Alias: None
Product: OpenLDAP
Classification: Unclassified
Component: slapd (show other issues)
Version: unspecified
Hardware: All All
: --- normal
Target Milestone: ---
Assignee: OpenLDAP project
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-03-28 04:58 UTC by armon.dadgar@gmail.com
Modified: 2014-12-11 01:01 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description armon.dadgar@gmail.com 2014-03-28 04:58:51 UTC
Full_Name: Armon Dadgar
Version: 
OS: 
URL: ftp://ftp.openldap.org/incoming/
Submission from: (NULL) (50.78.97.10)


I have an issue where when iterating over data and deleting keys, the cursor
will incorrectly skip rows, and then returns an early NOTFOUND even though more
records still exist.

I have a gist demonstrating this: https://gist.github.com/armon/9825666

I am using small Go wrapper around LMDB, version 0.9.11.

The expected behavior is to numbers 1..91 printed. However, when I run it,
it gets to 16 and returns no further rows.
Comment 1 Howard Chu 2014-03-28 20:56:25 UTC
armon.dadgar@gmail.com wrote:
> Full_Name: Armon Dadgar
> Version:
> OS:
> URL: ftp://ftp.openldap.org/incoming/
> Submission from: (NULL) (50.78.97.10)
>
>
> I have an issue where when iterating over data and deleting keys, the cursor
> will incorrectly skip rows, and then returns an early NOTFOUND even though more
> records still exist.
>
> I have a gist demonstrating this: https://gist.github.com/armon/9825666
>
> I am using small Go wrapper around LMDB, version 0.9.11.
>
> The expected behavior is to numbers 1..91 printed. However, when I run it,
> it gets to 16 and returns no further rows.
>
>
Thanks for the report. Fixed now in mdb.master.

-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/

Comment 2 Howard Chu 2014-03-29 04:35:39 UTC
changed notes
changed state Open to Test
moved from Incoming to Software Bugs
Comment 3 Hallvard Furuseth 2014-03-30 21:56:39 UTC
hyc@symas.com writes:
> Thanks for the report. Fixed now in mdb.master.

That fix breaks test052-memberof with pagesize 1024. Unrealistic, but
it's not a MAXKEYSIZE issue so it should not have made a difference.

Tested with RE24 9e22d8f2cd5ed10cd1dd22acbd729a49e5ecc1ab
+ mdb.master~ or mdb.master ac3acc121855e4297d597a4d71e9863e1e1e2450
+ this patch:

diff --git a/libraries/liblmdb/lmdb.h b/libraries/liblmdb/lmdb.h
index 2ebd43f..3f4a167 100644
--- a/libraries/liblmdb/lmdb.h
+++ b/libraries/liblmdb/lmdb.h
@@ -411,4 +411,4 @@ typedef enum MDB_cursor_op {
 	/** Too big key/data, key is empty, or wrong DUPFIXED size */
-#define MDB_BAD_VALSIZE		(-30781)
-#define MDB_LAST_ERRCODE	MDB_BAD_VALSIZE
+#define MDB_BAD_VALSIZE  (abort(),-30781) /* we don't hit MDB_MAXKEYSIZE */
+#define MDB_LAST_ERRCODE (-30781)
 /** @} */
diff --git a/libraries/liblmdb/mdb.c b/libraries/liblmdb/mdb.c
index 7e29058..ad9bfc6 100644
--- a/libraries/liblmdb/mdb.c
+++ b/libraries/liblmdb/mdb.c
@@ -405,3 +405,3 @@ static txnid_t mdb_debug_start;
 #ifndef MDB_MAXKEYSIZE
-#define MDB_MAXKEYSIZE	 511
+#define MDB_MAXKEYSIZE 400 /* max-max is 446 for psize=1024 on 64-bit hosts */
 #endif
@@ -3536,3 +3536,3 @@ mdb_env_open2(MDB_env *env)
 		newenv = 1;
-		env->me_psize = env->me_os_psize;
+		env->me_psize = 1024;
 		if (env->me_psize > MAX_PAGESIZE)

-- 
Hallvard

Comment 4 Hallvard Furuseth 2014-03-30 22:35:31 UTC
Sorry, forgot the crash info.

mdb.c:7419: Assertion 'NUMKEYS(mc->mc_pg[ptop]) > 1' failed in mdb_rebalance()

(gdb) p *mc->mc_pg[ptop]
$1 = {
  mp_p = {p_pgno = 24, p_next = 0x18}, mp_pad = 0, mp_flags = 17, 
  mp_pb = {pb = {pb_lower = 18, pb_upper = 1016}, pb_pages = 66584594}, 
  mp_ptrs = {1016}
}
(gdb) info locals
node = 0x0
rc = -268443168
ptop = 0
minkeys = 1
mn = {
  mc_next = 0x7ffff5ce0010, mc_backup = 0x7fffe41013a0, mc_xcursor = 0x0, 
  mc_txn = 0x7fffe42c9700, mc_dbi = 6, mc_db = 0x7fffe42c9890, 
  mc_dbx = 0x7fffe42c3fb0, mc_dbflag = 0x7fffe42c3fb0 "\002", 
  mc_snum = 2, mc_top = 1, mc_flags = 65, 
  mc_pg = {0x7fffe4102840, 0x7fffe8109670, 0x2efffe300, 0x7fffefffe0c0, 0x7fffefffe0a0, 0x7fffefffe2c0, 
    0x7fffefffe2c0, 0x53e7cd, 0x7fffefffe1c0, 0x7fffe40019f0, 0x7fffe41013a0, 0x7fffefffe2c0, 0x30c3669909, 
    0xfbad8001, 0x7fffefffe2c0, 0x7fffefffe2c0, 0x7fffefffe2c0, 0x7fffefffe130, 0x7fffefffe2d0, 0x7fffefffe180, 
    0x49c7ca, 0x7fffefffe140, 0x7fffefffe1dc, 0x1100000000, 0x0, 0x7fffefffe2d0, 0x7fffe4101528, 0x7fffefffe180, 0x1, 
    0x7fffe81099d4, 0x7fffefffe260, 0x100000008}, 
  mc_ki = {5416, 58384, 32767, 0, 39380, 59408, 32767, 0, 39364, 59408, 32767, 0, 39400, 59408, 32767, 0, 39332, 
    59408, 32767, 0, 68, 0, 0, 0, 15369, 50024, 48, 0, 57792, 61439, 32767, 0}
}
oldki = 0
__FUNCTION__ = "mdb_rebalance"
(gdb) p *mc
$2 = {
  mc_next = 0x0, mc_backup = 0x0, mc_xcursor = 0x7fffe4101528, 
  mc_txn = 0x7fffe42c9700, mc_dbi = 6, mc_db = 0x7fffe42c9890,
  mc_dbx = 0x7fffe42c3fb0, mc_dbflag = 0x7fffe42cb3e6 "\t\n\n", 
  mc_snum = 2, mc_top = 1, mc_flags = 65, 
  mc_pg = {0x7fffe4102840, 0x7fffe8109670, 0x0, 0x40, 0x44, 0x0, 0x6a206e616d726568, 0x6f65703d756f2c72, 
    0x653d63642c656c70, 0x642c656c706d6178, 0x302e006d6f633d63, 0x30235a3632353631, 0x2d1, 0x7fffe4000338, 0x81, 
    0x7fffe4102c40, 0x7fffe42c7860, 0x20, 0x64, 0x0, 0x4, 0x1a, 0x19, 0x10, 0xe, 0xd, 0xb, 0xa, 0x9, 0x140, 0x84, 
    0x0}, 
  mc_ki = {0, 5, 0, 0, 22160, 63214, 32767, 0, 32769, 0, 0, 0, 64592, 70, 0, 0, 22160, 63214, 32767, 0, 32775, 0, 0, 
    0, 63424, 70, 0, 0, 22160, 63214, 32767, 0}
}

Backtrace:
#2  mdb_assert_fail ()
#3  mdb_rebalance (mc=0x7fffe41013a0) at mdb.c:7419
#4  mdb_cursor_del0 (mc=0x7fffe41013a0) at mdb.c:7496
#5  mdb_cursor_del (mc=0x7fffe41013a0, flags=0) at mdb.c:6360
#6  mdb_idl_delete_keys (be, cursor=0x7fffe41013a0, keys, id=7) at idl.c:608
#7  indexer (op=0x7fffe4000950, txn, ai, ad=0x838180, atname=0x837fc8, vals=0x7fffe40019f0, id=7, opid=2, mask=1814) at index.c:258
#8  index_at_values (op=0x7fffe4000950, txn=0x7fffe42c9700, ad, type=0x837f60, tags=0x8381a0, vals=0x7fffe40019f0, id=7, opid=2) at index.c:337
#9  mdb_index_values (op, txn, desc, vals, id, opid) at index.c:386
#10 mdb_index_entry (op=0x7fffe4000950, txn=0x7fffe42c9700, opid=2, e=0x7fffe40017d0) at index.c:558
#11 mdb_delete (op=0x7fffe4000950, rs=0x7fffefffe910) at delete.c:347
#12 overlay_op_walk (op=0x7fffe4000950, rs=0x7fffefffe910, which=op_delete, oi=0x7fffe81036a0, on=0x0) at backover.c:671
#13 over_op_func (op=0x7fffe4000950, rs, which) at backover.c:723
#14 fe_op_delete (op=0x7fffe4000950, rs=0x7fffefffe910) at delete.c:174
#15 do_delete (op=0x7fffe4000950, rs=0x7fffefffe910) at delete.c:95
#16 connection_operation (ctx=0x7fffefffea70, arg_v=0x7fffe4000950) at connection.c:1155
#17 connection_read_thread (ctx=0x7fffefffea70, argv) at connection.c:1291
#18 ldap_int_thread_pool_wrapper (xpool=0x83b3b0) at tpool.c:688

Test output:
Running ldapadd to build slapd config database...
Running ldapadd to build slapd database...
Search the entire database...
Running ldapmodify to add a member...
Re-search the entire database...
Running ldapmodify to rename a member...
Re-search the entire database...
Running ldapmodify to rename a group...
Re-search the entire database...
Running ldapmodify to add self...
Re-search the entire database...
Running ldapdelete to remove a member...
Re-search the entire database...
Running ldapdelete to remove a group...
Re-search the entire database...
Adding groups with MAY member type schemas...

Log ends with:
533898e1 => index_entry_del( 7, "cn=Jessica Rabbit,ou=People,dc=example,dc=com" )
533898e1 mdb_idl_delete_keys: 7 [860433ad]
533898e1 mdb_idl_delete_keys: 7 [00000000]
533898e1 mdb_idl_delete_keys: 7 [aaacba45]
533898e1 mdb_idl_delete_keys: 7 [e10d2617]

-- 
Hallvard

Comment 5 Howard Chu 2014-03-31 20:50:12 UTC
Hallvard Breien Furuseth wrote:
> Sorry, forgot the crash info.

Thanks, confirmed. In a nested rebalance, doing a page merge with the neighbor 
on the left was discarding the rebalanced state. Fixed now in mdb.master
>
> mdb.c:7419: Assertion 'NUMKEYS(mc->mc_pg[ptop]) > 1' failed in mdb_rebalance()
>
> (gdb) p *mc->mc_pg[ptop]
> $1 = {
>    mp_p = {p_pgno = 24, p_next = 0x18}, mp_pad = 0, mp_flags = 17,
>    mp_pb = {pb = {pb_lower = 18, pb_upper = 1016}, pb_pages = 66584594},
>    mp_ptrs = {1016}
> }
> (gdb) info locals
> node = 0x0
> rc = -268443168
> ptop = 0
> minkeys = 1
> mn = {
>    mc_next = 0x7ffff5ce0010, mc_backup = 0x7fffe41013a0, mc_xcursor = 0x0,
>    mc_txn = 0x7fffe42c9700, mc_dbi = 6, mc_db = 0x7fffe42c9890,
>    mc_dbx = 0x7fffe42c3fb0, mc_dbflag = 0x7fffe42c3fb0 "\002",
>    mc_snum = 2, mc_top = 1, mc_flags = 65,
>    mc_pg = {0x7fffe4102840, 0x7fffe8109670, 0x2efffe300, 0x7fffefffe0c0, 0x7fffefffe0a0, 0x7fffefffe2c0,
>      0x7fffefffe2c0, 0x53e7cd, 0x7fffefffe1c0, 0x7fffe40019f0, 0x7fffe41013a0, 0x7fffefffe2c0, 0x30c3669909,
>      0xfbad8001, 0x7fffefffe2c0, 0x7fffefffe2c0, 0x7fffefffe2c0, 0x7fffefffe130, 0x7fffefffe2d0, 0x7fffefffe180,
>      0x49c7ca, 0x7fffefffe140, 0x7fffefffe1dc, 0x1100000000, 0x0, 0x7fffefffe2d0, 0x7fffe4101528, 0x7fffefffe180, 0x1,
>      0x7fffe81099d4, 0x7fffefffe260, 0x100000008},
>    mc_ki = {5416, 58384, 32767, 0, 39380, 59408, 32767, 0, 39364, 59408, 32767, 0, 39400, 59408, 32767, 0, 39332,
>      59408, 32767, 0, 68, 0, 0, 0, 15369, 50024, 48, 0, 57792, 61439, 32767, 0}
> }
> oldki = 0
> __FUNCTION__ = "mdb_rebalance"
> (gdb) p *mc
> $2 = {
>    mc_next = 0x0, mc_backup = 0x0, mc_xcursor = 0x7fffe4101528,
>    mc_txn = 0x7fffe42c9700, mc_dbi = 6, mc_db = 0x7fffe42c9890,
>    mc_dbx = 0x7fffe42c3fb0, mc_dbflag = 0x7fffe42cb3e6 "\t\n\n",
>    mc_snum = 2, mc_top = 1, mc_flags = 65,
>    mc_pg = {0x7fffe4102840, 0x7fffe8109670, 0x0, 0x40, 0x44, 0x0, 0x6a206e616d726568, 0x6f65703d756f2c72,
>      0x653d63642c656c70, 0x642c656c706d6178, 0x302e006d6f633d63, 0x30235a3632353631, 0x2d1, 0x7fffe4000338, 0x81,
>      0x7fffe4102c40, 0x7fffe42c7860, 0x20, 0x64, 0x0, 0x4, 0x1a, 0x19, 0x10, 0xe, 0xd, 0xb, 0xa, 0x9, 0x140, 0x84,
>      0x0},
>    mc_ki = {0, 5, 0, 0, 22160, 63214, 32767, 0, 32769, 0, 0, 0, 64592, 70, 0, 0, 22160, 63214, 32767, 0, 32775, 0, 0,
>      0, 63424, 70, 0, 0, 22160, 63214, 32767, 0}
> }
>
> Backtrace:
> #2  mdb_assert_fail ()
> #3  mdb_rebalance (mc=0x7fffe41013a0) at mdb.c:7419
> #4  mdb_cursor_del0 (mc=0x7fffe41013a0) at mdb.c:7496
> #5  mdb_cursor_del (mc=0x7fffe41013a0, flags=0) at mdb.c:6360
> #6  mdb_idl_delete_keys (be, cursor=0x7fffe41013a0, keys, id=7) at idl.c:608
> #7  indexer (op=0x7fffe4000950, txn, ai, ad=0x838180, atname=0x837fc8, vals=0x7fffe40019f0, id=7, opid=2, mask=1814) at index.c:258
> #8  index_at_values (op=0x7fffe4000950, txn=0x7fffe42c9700, ad, type=0x837f60, tags=0x8381a0, vals=0x7fffe40019f0, id=7, opid=2) at index.c:337
> #9  mdb_index_values (op, txn, desc, vals, id, opid) at index.c:386
> #10 mdb_index_entry (op=0x7fffe4000950, txn=0x7fffe42c9700, opid=2, e=0x7fffe40017d0) at index.c:558
> #11 mdb_delete (op=0x7fffe4000950, rs=0x7fffefffe910) at delete.c:347
> #12 overlay_op_walk (op=0x7fffe4000950, rs=0x7fffefffe910, which=op_delete, oi=0x7fffe81036a0, on=0x0) at backover.c:671
> #13 over_op_func (op=0x7fffe4000950, rs, which) at backover.c:723
> #14 fe_op_delete (op=0x7fffe4000950, rs=0x7fffefffe910) at delete.c:174
> #15 do_delete (op=0x7fffe4000950, rs=0x7fffefffe910) at delete.c:95
> #16 connection_operation (ctx=0x7fffefffea70, arg_v=0x7fffe4000950) at connection.c:1155
> #17 connection_read_thread (ctx=0x7fffefffea70, argv) at connection.c:1291
> #18 ldap_int_thread_pool_wrapper (xpool=0x83b3b0) at tpool.c:688
>
> Test output:
> Running ldapadd to build slapd config database...
> Running ldapadd to build slapd database...
> Search the entire database...
> Running ldapmodify to add a member...
> Re-search the entire database...
> Running ldapmodify to rename a member...
> Re-search the entire database...
> Running ldapmodify to rename a group...
> Re-search the entire database...
> Running ldapmodify to add self...
> Re-search the entire database...
> Running ldapdelete to remove a member...
> Re-search the entire database...
> Running ldapdelete to remove a group...
> Re-search the entire database...
> Adding groups with MAY member type schemas...
>
> Log ends with:
> 533898e1 => index_entry_del( 7, "cn=Jessica Rabbit,ou=People,dc=example,dc=com" )
> 533898e1 mdb_idl_delete_keys: 7 [860433ad]
> 533898e1 mdb_idl_delete_keys: 7 [00000000]
> 533898e1 mdb_idl_delete_keys: 7 [aaacba45]
> 533898e1 mdb_idl_delete_keys: 7 [e10d2617]
>


-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/

Comment 6 Hallvard Furuseth 2014-04-01 15:36:40 UTC
On Fri, 2014-03-28 at 04:58 +0000, armon.dadgar@gmail.com wrote:
> I have a gist demonstrating this: https://gist.github.com/armon/9825666

Some notes about this code:
    // LMDB will return EINVAL(22) for the GET_CURRENT op if
    // there is no further keys. We treat this as no more
    // keys being found.
    if num, ok := err.(mdb.Errno); ok && num == 22 {
        println("errno 22")
        err = mdb.NotFound
    }

The value of EINVAL is OS-dependent, it need not be 22.

EINVAL means you did something wrong, it's a bad idea to
assume it means any particular wrong thing.  For one thing,
mdb might not keep catching that error.  I'd follow up
with a test check that there are indeed no more keys.


That said, mdb_cursor_del() followed by MDB_GET_CURRENT
seems to me a sensible thing to want to do.  So I think
MDB_GET_CURRENT should return MDB_NOTFOUND here.  And maybe
also after cursor_next() returned MDB_NOTFOUND, and on an
empty DB.  OTOH maybe cursors would need to keep more state
to tell this from user errors which should get EINVAL.


Comment 7 armon.dadgar@gmail.com 2014-04-01 18:22:43 UTC
I agree that it would be great to have MDB_NOTFOUND returned
instead of EINVAL. For my purposes, I needed a work around to the
issue, and this seemed to work. Additionally because I’m in Go and not
C, I cannot easily make use of the system definition for EINVAL.

Best Regards,
Armon Dadgar

From: Hallvard Breien Furuseth h.b.furuseth@usit.uio.no
Reply: Hallvard Breien Furuseth h.b.furuseth@usit.uio.no
Date: April 1, 2014 at 8:36:43 AM
To: armon.dadgar@gmail.com armon.dadgar@gmail.com
Cc: openldap-its@openldap.org openldap-its@openldap.org
Subject:  Re: (ITS#7829) MDB mdb_cursor_del causes records to be skipped  

On Fri, 2014-03-28 at 04:58 +0000, armon.dadgar@gmail.com wrote:  
> I have a gist demonstrating this: https://gist.github.com/armon/9825666  

Some notes about this code:  
// LMDB will return EINVAL(22) for the GET_CURRENT op if  
// there is no further keys. We treat this as no more  
// keys being found.  
if num, ok := err.(mdb.Errno); ok && num == 22 {  
println("errno 22")  
err = mdb.NotFound  
}  

The value of EINVAL is OS-dependent, it need not be 22.  

EINVAL means you did something wrong, it's a bad idea to  
assume it means any particular wrong thing. For one thing,  
mdb might not keep catching that error. I'd follow up  
with a test check that there are indeed no more keys.  


That said, mdb_cursor_del() followed by MDB_GET_CURRENT  
seems to me a sensible thing to want to do. So I think  
MDB_GET_CURRENT should return MDB_NOTFOUND here. And maybe  
also after cursor_next() returned MDB_NOTFOUND, and on an  
empty DB. OTOH maybe cursors would need to keep more state  
to tell this from user errors which should get EINVAL.  

Comment 8 Hallvard Furuseth 2014-04-02 09:26:20 UTC
Howard Chu writes:
> Thanks, confirmed. In a nested rebalance, doing a page merge with the
> neighbor on the left was discarding the rebalanced state. Fixed now in
> mdb.master

Nope. 69edafe28aef03b965c3d911be9ab8e340f914e1 "ITS#7829 more for
prev commit" breaks test054-syncreplication-parallel-load in RE24
(with normal pagesize), every 2nd run or so.

Running ./scripts/test054-syncreplication-parallel-load for mdb...
running defines.sh
Starting provider slapd on TCP/IP port 9011...
Using ldapsearch to check that provider slapd is running...
Using ldapadd to create the context prefix entry in the provider...
Starting consumer slapd on TCP/IP port 9014...
Using ldapsearch to check that consumer slapd is running...
Using ldapadd to populate the provider directory...
Waiting 7 seconds for syncrepl to receive changes...
Stopping the provider, sleeping 10 seconds and restarting it...
Using ldapsearch to check that provider slapd is running...
Waiting 10 seconds to let the system catch up
Using ldapmodify to modify provider directory...
ldapmodify failed (80)!

slapd.1.log says:

533bbaa2 mdb_modrdn: new ndn=cn=rosco p. coltrane,ou=retired,ou=people,dc=example,dc=com
533bbaa2 => mdb_dn2id("cn=rosco p. coltrane,ou=retired,ou=people,dc=example,dc=com")
533bbaa2 <= mdb_dn2id: get failed: MDB_NOTFOUND: No matching key/data pair found (-30798)
533bbaa2 => mdb_dn2id_delete 0x2a
533bbaa2 <= mdb_dn2id_delete 0x2a: -30798
533bbaa2 <=- mdb_modrdn: dn2id del failed: MDB_NOTFOUND: No matching key/data pair found (-30798)
533bbaa2 send_ldap_result: conn=1002 op=8 p=3
533bbaa2 send_ldap_result: err=80 matched="" text="DN index delete fail"


This dn2id.c assert() would catch it. Passing -MDB_SET to catch it in
a patched liblmdb before that.

diff --git a/servers/slapd/back-mdb/dn2id.c b/servers/slapd/back-mdb/dn2id.c
index ceacb17..8cb9d91 100644
--- a/servers/slapd/back-mdb/dn2id.c
+++ b/servers/slapd/back-mdb/dn2id.c
@@ -243,3 +243,4 @@ mdb_dn2id_delete(
 		do {
-			rc = mdb_cursor_get( mc, &key, &data, MDB_SET );
+			rc = mdb_cursor_get( mc, &key, &data, -MDB_SET );
+			assert(!rc);
 			if ( !rc ) {

mdb.c fails at 1st assert, the rest is for handling -MDB_SET:

diff --git a/libraries/liblmdb/mdb.c b/libraries/liblmdb/mdb.c
index 10a8358..71b0b93 100644
--- a/libraries/liblmdb/mdb.c
+++ b/libraries/liblmdb/mdb.c
@@ -5348,2 +5348,3 @@ mdb_cursor_set(MDB_cursor *mc, MDB_val *key, MDB_val *data,
 		rc = mc->mc_dbx->md_cmp(key, &nodekey);
+		mdb_cassert(mc, op != (MDB_cursor_op)-MDB_SET || rc >= 0 || mc->mc_top);
 		if (rc == 0) {
@@ -5460,3 +5461,3 @@ set1:
 		if (F_ISSET(leaf->mn_flags, F_DUPDATA)) {
-			if (op == MDB_SET || op == MDB_SET_KEY || op == MDB_SET_RANGE) {
+			if (op == MDB_SET || op == (MDB_cursor_op)-MDB_SET || op == MDB_SET_KEY || op == MDB_SET_RANGE) {
 				rc = mdb_cursor_first(&mc->mc_xcursor->mx_cursor, data, NULL);
@@ -5651,2 +5652,3 @@ mdb_cursor_get(MDB_cursor *mc, MDB_val *key, MDB_val *data,
 	case MDB_SET_RANGE:
+	case (MDB_cursor_op)-MDB_SET:
 		if (key == NULL) {
@@ -5747,2 +5749,3 @@ fetchm:
 		DPRINTF(("unhandled/unimplemented cursor operation %u", op));
+		mdb_cassert(mc, 0); /* just checking that we never get here */
 		rc = EINVAL;

#2  mdb_assert_fail ()
#3  mdb_cursor_set (mc=0x7fabb4102bb0, key=0x7fabbf276260,
    data=0x7fabbf276250, op=4294967281, exactp=0x7fabbf2761f0) at mdb.c:5349
#4  mdb_cursor_get (mc=0x7fabb4102bb0, key=0x7fabbf276260,
    data=0x7fabbf276250, op=4294967281) at mdb.c:5657
#5  mdb_dn2id_delete (op=0x7fabb4000960, mc=0x7fabb4102bb0, id=42, nsubs=1)
    at dn2id.c:244
#6  mdb_modrdn (op=0x7fabb4000960, rs=0x7fabbf276910) at modrdn.c:476
#7  overlay_op_walk (op=0x7fabb4000960, rs=0x7fabbf276910, which=op_modrdn,
    oi=0x1c2c120, on=0x0) at backover.c:671
#8  over_op_func (op=0x7fabb4000960, rs, which) at backover.c:723
#9  fe_op_modrdn (op=0x7fabb4000960, rs=0x7fabbf276910) at modrdn.c:314
#10 do_modrdn (op=0x7fabb4000960, rs=0x7fabbf276910) at modrdn.c:186
#11 connection_operation (ctx=0x7fabbf276a70, arg_v=0x7fabb4000960)
    at connection.c:1155
#12 connection_read_thread (ctx=0x7fabbf276a70, argv) at connection.c:1291
#13 ldap_int_thread_pool_wrapper (xpool=0x1bd7370) at tpool.c:688

(gdb) frame 3
#3  mdb_cursor_set (mc=0x7fabb4102bb0, key=0x7fabbf276260,
    data=0x7fabbf276250, op=4294967281, exactp=0x7fabbf2761f0) at mdb.c:5349
5349            mdb_cassert(mc, op != (MDB_cursor_op)-MDB_SET || rc >= 0 || mc->mc_top);
(gdb) info locals
nodekey = {mv_size = 8, mv_data = 0x7fabb01092da}
rc = -8
mp = 0x7fabb0108310
leaf = 0x7fabb01092d2
__FUNCTION__ = "mdb_cursor_set"
(gdb) set output-radix 16
(gdb) p *mp
$1 = {mp_p = {p_pgno = 0x51, p_next = 0x51}, mp_pad = 0x0, mp_flags = 0x12,
      mp_pb = {pb = {pb_lower = 0x44, pb_upper = 0xa2a}, pb_pages = 0xa2a0044},
      mp_ptrs = {0xfc2}}
(gdb) p *leaf
$2 = {mn_lo = 0x2e, mn_hi = 0x0, mn_flags = 0x0, mn_ksize = 0x8,
      mn_data = "\020"}
(gdb) p *(MDB_ID*)key->mv_data
$3 = 0x8
(gdb) p *(MDB_ID*)nodekey.mv_data
$4 = 0x10

-- 
Hallvard

Comment 9 Hallvard Furuseth 2014-04-03 04:57:39 UTC
I wrote:
> Nope. 69edafe28aef03b965c3d911be9ab8e340f914e1 "ITS#7829 more for
> prev commit" breaks test054-syncreplication-parallel-load in RE24
> (with normal pagesize), every 2nd run or so.

Or rather fca18d2586259c0e802d0276f86be696863b1905, not the ID
I was using when bisecting:-)


Comment 10 Hallvard Furuseth 2014-04-03 16:18:53 UTC
Another mdb_cursor_del() issue: If the next operation moves/peeks
at the cursor position, it does not always reset the C_DEL flag:

* mdb_cursor_put(,&existing_key,, MDB_APPEND or MDB_NOOVERWRITE)
  works like mdb_cursor_get(, &existing_key,, MDB_SET) but does
  not clear C_DEL. So MDB_NEXT after that stays at &existing_key.

  The enclosed program illustrates this: the MDB_NEXTs give
  different results after the cursor was positioned at "f".

* I'm guessing mdb_cursor_count() should reset C_DEL on the main
  cursor:

  The mdb_cursor_del() semantics seems to be something like "the
  cursor moves to the next item, however if the next operation is
  an MDB_NEXT* operation then it also returns that item.  So the
  manner you peek at the cursor position determines whether the
  the next item is "next" or "current".

  But I haven't thought carefully about C_DEL vs. DUPSORT cursors.

BTW,
  mc->mc_flags &= ~C_DEL;
would be a bit simpler than
  if (mc->mc_flags & C_DEL) mc->mc_flags ^= C_DEL;

################################################################

#include <stdio.h>
#include <stdlib.h>
#include "lmdb.h"
#define E(expr) CHECK((rc = (expr)) == MDB_SUCCESS, #expr)
#define CHECK(test, msg) ((test) ? (void)0 : ((void)fprintf(stderr, \
    "%s:%d: %s: %s\n", __FILE__, __LINE__, msg, mdb_strerror(rc)), abort()))

int main(void)
{
    int rc, i;
    char ch, *act;
    MDB_env *env;
    MDB_txn *txn;
    MDB_dbi dbi;
    MDB_cursor *cursor;
    MDB_cursor_op op;
    remove("test.mdb");
    E(mdb_env_create(&env));
    E(mdb_env_open(env, "test.mdb", MDB_NOSUBDIR, 0664));
    E(mdb_txn_begin(env, NULL, 0, &txn));
    E(mdb_open(txn, NULL, 0, &dbi));
    E(mdb_cursor_open(txn, dbi, &cursor));

    for (op=MDB_NEXT, act="NEXT";; op=MDB_GET_CURRENT, act="GET_CURRENT") {
        MDB_val key, data = {0, ""};
        for (ch = 'a'; ch <= 'h'; ch++)
            E(mdb_cursor_put(cursor, &(MDB_val){1, &ch}, &data, 0));
        for (i = 0; i < 2; i++) {
            E(mdb_cursor_get(cursor, &key, NULL, MDB_FIRST));
            E(mdb_cursor_del(cursor, 0));
            if (i == 0) {
                E(mdb_cursor_get(cursor, &(MDB_val){1, "f"}, NULL, MDB_SET));
            } else {
                rc = mdb_cursor_put(cursor, &(MDB_val){1, "f"}, &data,
                    MDB_NOOVERWRITE); /* C_DEL vs. MDB_NEXT bug? */
                CHECK(rc == MDB_KEYEXIST, "no-overwrite");
            }
            rc = mdb_cursor_get(cursor, &key, &data, op);
            if (rc == MDB_SUCCESS)
                printf("MDB_%s = %c\n", act, *(char *)key.mv_data);
            else
                printf("MDB_%s = %s\n", act, mdb_strerror(rc));
        }
        if (op == MDB_GET_CURRENT)
            break;
    }

    mdb_txn_abort(txn);
    mdb_env_close(env);
    return 0;
}

Comment 11 OpenLDAP project 2014-08-01 21:04:51 UTC
fixed in mdb.master
Comment 12 Quanah Gibson-Mount 2014-12-11 01:01:53 UTC
changed state Test to Closed