openldap-bugs

openldap-bugs@openldap.org

20960 discussions

Re: (ITS#8430) Improved handling for large number of databases
by juerg.bircher＠gmail.com 27 May '16

27 May '16

I am switching to gmail as in outlook my name uses Umlaute which results in base64 encoded mails! Strangely the mailing list openldap-technical can handle it and openldap-its cannot! Here the missing disclaimer for the patch: The attached patch file is derived from OpenLDAP Software. All of the modifications to OpenLDAP Software represented in the following patch(es) were developed by Juerg Bircher juerg.bircher(a)helmedica.com. I have not assigned rights and/or interest in this work to any party. I, Juerg Bircher, hereby place the following modifications to OpenLDAP Software (and only these modifications) into the public domain. Hence, these modifications may be freely used and/or redistributed for any purpose with or without attribution and/or other notice.

1 0

Re: (ITS#8431) LMDB: Access newly opened database from another transaction
by h.b.furuseth＠usit.uio.no 27 May '16

27 May '16

This works as intended. Each transaction is atomic, and shall see neither DBIs nor data which were committed after the transaction began. Cross-txn mdb_dbi_open() is in any case hairy to get right, we screwed it up several times before arriving at the now-obvious semantics. An application can open and cache its DBIs in a separate startup transaction, then commit it and start new transactions. -- Hallvard

1 0

Re: (ITS#8430) Improved handling for large number of databases
by juerg.bircher＠helmedica.com 27 May '16

27 May '16

SGVyZSB0aGUgbWlzc2luZyBkaXNjbGFpbWVyIGZvciB0aGUgcGF0Y2g6DQoNClRoZSBhdHRhY2hl ZCBwYXRjaCBmaWxlIGlzIGRlcml2ZWQgZnJvbSBPcGVuTERBUCBTb2Z0d2FyZS4gQWxsIG9mIHRo ZSBtb2RpZmljYXRpb25zIHRvIE9wZW5MREFQIFNvZnR3YXJlIHJlcHJlc2VudGVkIGluIHRoZSBm b2xsb3dpbmcgcGF0Y2goZXMpIHdlcmUgZGV2ZWxvcGVkIGJ5IEp1ZXJnIEJpcmNoZXIganVlcmcu YmlyY2hlckBoZWxtZWRpY2EuY29tLiBJIGhhdmUgbm90IGFzc2lnbmVkIHJpZ2h0cyBhbmQvb3Ig aW50ZXJlc3QgaW4gdGhpcyB3b3JrIHRvIGFueSBwYXJ0eS4NCg0KSSwgSnVlcmcgQmlyY2hlciwg aGVyZWJ5IHBsYWNlIHRoZSBmb2xsb3dpbmcgbW9kaWZpY2F0aW9ucyB0byBPcGVuTERBUCBTb2Z0 d2FyZSAoYW5kIG9ubHkgdGhlc2UgbW9kaWZpY2F0aW9ucykgaW50byB0aGUgcHVibGljIGRvbWFp bi4gSGVuY2UsIHRoZXNlIG1vZGlmaWNhdGlvbnMgbWF5IGJlIGZyZWVseSB1c2VkIGFuZC9vciBy ZWRpc3RyaWJ1dGVkIGZvciBhbnkgcHVycG9zZSB3aXRoIG9yIHdpdGhvdXQgYXR0cmlidXRpb24g YW5kL29yIG90aGVyIG5vdGljZS4NCg0K

1 0

Re: (ITS#8430) Improved handling for large number of databases
by juerg.bircher＠helmedica.com 27 May '16

27 May '16

DQpEaXNjbGFpbWVyDQoNCg0KDQpUaGUgYXR0YWNoZWQgcGF0Y2ggZmlsZSBpcyBkZXJpdmVkIGZy b20gT3BlbkxEQVAgU29mdHdhcmUuIEFsbCBvZiB0aGUgbW9kaWZpY2F0aW9ucyB0byBPcGVuTERB UCBTb2Z0d2FyZSByZXByZXNlbnRlZCBpbiB0aGUgZm9sbG93aW5nIHBhdGNoKGVzKSB3ZXJlIGRl dmVsb3BlZCBieSBKdWVyZyBCaXJjaGVyIGp1ZXJnLmJpcmNoZXJAaGVsbWVkaWNhLmNvbS4gSSBo YXZlIG5vdCBhc3NpZ25lZCByaWdodHMgYW5kL29yIGludGVyZXN0IGluIHRoaXMgd29yayB0byBh bnkgcGFydHkuDQoNCkksIEp1ZXJnIEJpcmNoZXIsIGhlcmVieSBwbGFjZSB0aGUgZm9sbG93aW5n IG1vZGlmaWNhdGlvbnMgdG8gT3BlbkxEQVAgU29mdHdhcmUgKGFuZCBvbmx5IHRoZXNlIG1vZGlm aWNhdGlvbnMpIGludG8gdGhlIHB1YmxpYyBkb21haW4uIEhlbmNlLCB0aGVzZSBtb2RpZmljYXRp b25zIG1heSBiZSBmcmVlbHkgdXNlZCBhbmQvb3IgcmVkaXN0cmlidXRlZCBmb3IgYW55IHB1cnBv c2Ugd2l0aCBvciB3aXRob3V0IGF0dHJpYnV0aW9uIGFuZC9vciBvdGhlciBub3RpY2UuDQoNCg0K DQpPbiAyNy8wNS8xNiAxMzoxNiwgIkhvd2FyZCBDaHUiIDxoeWNAc3ltYXMuY29tPiB3cm90ZToN Cg0KPmp1ZXJnLmJpcmNoZXJAaGVsbWVkaWNhLmNvbSB3cm90ZToNCj4+IEZ1bGxfTmFtZTogSnVl cmcgQmlyY2hlcg0KPj4gVmVyc2lvbjogbG1kYiAobWFzdGVyKQ0KPj4gT1M6IE1hY09TIC8gTGlu dXgNCj4+IFVSTDogZnRwOi8vZnRwLm9wZW5sZGFwLm9yZy9pbmNvbWluZy9KdWVyZ19CaXJjaGVy XzE2MDUyNy1JbXByb3ZlZC1oYW5kbGluZy1mb3ItbGFyZ2UtbnVtYmVyLW9mLWRhdGFiYXNlcy5w YXRjaA0KPj4gU3VibWlzc2lvbiBmcm9tOiAoTlVMTCkgKDE3OC44Mi4zNy4xOTUpDQo+DQo+VGhh bmtzIGZvciB0aGUgcGF0Y2gsIGJ1dCBmb3IgYSBjaGFuZ2Ugb2YgdGhpcyBzaXplIHlvdSBhbHNv IG5lZWQgdG8gaW5jbHVkZSANCj5hbiBJUCBSaWdodHMgbm90aWNlLCBhcyBkb2N1bWVudGVkIG9u IHRoZSBDb250cmlidXRpbmcgZ3VpZGVsaW5lcy4NCj4+DQo+Pg0KPj4gVGhlcmUgaXMgYSBpbmNy ZWFzZWQgcGVyZm9ybWFuY2UgcGVuYWx0eSB0aGUgbW9yZSBkYXRhYmFzZXMgYXJlIGNyZWF0ZWQg d2l0aGluDQo+PiB0aGUgc2FtZSBlbnZpcm9ubWVudC4gSSB3YXMgbG9va2luZyBmb3IgYSB3YXkg dGhlIGltcHJvdmUgdGhhdCBieSBrZWVwaW5nIHRoZQ0KPj4gc2ltcGxpY2l0eSBvZiB0cmFja2lu ZyBkYXRhYmFzZXMgd2l0aGluIGEgbGlzdCB3aXRoIGRpcmVjdCBhY2Nlc3MgYnkgaW5kZXgNCj4+ IChNREJfZGJpKS4NCj4+IG1kYl9kYmlfb3BlbigpIGlzIGhvd2V2ZXIgbm90IGltcHJvdmVkIHdp dGggdGhlIGFzc3VtcHRpb24gdGhhdCB0aGUgZGF0YWJhc2UNCj4+IGhhbmRsZSAoZGJpKSBpcyBj YWNoZWQgaW4gdGhlIGFwcGxpY2F0aW9uLiBTbyBtZGJfZGJpX29wZW4oKSBzaG91bGQgaGFwcGVu IG9ubHkNCj4+IG9uY2UgZm9yIGVhY2ggZGF0YWJhc2UgZHVyaW5nIHRoZSBsaWZlIHRpbWUgb2Yg YW4gYXBwbGljYXRpb24uDQo+Pg0KPj4NCj4+IE9uZSBpc3N1ZSBpcyB0aGF0IG1kYl90eG5fYmVn aW4oKSAoZm9yIHJlYWQtb25seSB0cmFuc2FjdGlvbnMpIGNhbGxvYyB0aGUNCj4+IHNpemVvZihN REJfdHhuKSArIG1lX21heGRicyAqIHNpemVvZihNREJfZGIgKyAxKS4gVGhlIHBsdXMgMSBmb3Ig dGhlIGRiZmxhZ3MuDQo+PiBIb3dldmVyIGl0IGlzIHN1ZmZpY2llbnQgb25seSB0byBtYWxsb2Mg dGhhdCBzaXplIGFuZCBjbGVhciB0aGUgc2l6ZW9mKE1EQl90eG4pDQo+Pg0KPj4gICAgICBtZW1z ZXQodHhuLCAwLCBzaXplb2YoTURCX3R4bikNCj4+DQo+PiBBZnRlciB0aGF0IHRoZSBkYXRhIGJl eW9uZCB0aGUgTURCX3R4biBpcyBub3QgaW5pdGlhbGl6ZWQgd2hpY2ggaXMgb2sgZm9yIHRoZQ0K Pj4gbW9tZW50Lg0KPj4NCj4+IFRoZSBuZXh0IGltcHJvdmVtZW50IGhhcHBlbnMgaW4gbWRiX3R4 bl9yZW5ldzAoKSB3aGVyZSB0aGUgZGJmbGFncyBhcmUgb25seSBzZXQNCj4+IHRvIERCX1VOVVNF RCAoYSBuZXcgZmxhZykgZm9yIGVhY2ggZGF0YWJhc2UgY3VycmVudGx5IG9wZW5lZCBpbiB0aGUN Cj4+IGVudmlyb25tZW50Lg0KPj4NCj4+ICAgICAgbWVtc2V0KHR4bi0+bXRfZGJmbGFncywgREJf VU5VU0VELCB0eG4tPm10X251bWRicyk7DQo+Pg0KPj4gVGhlIGZvcm1lciBjb2RlIHVzZWQgdG8g dG8gbG9vcCB0aHJvdWdoIGVhY2ggZGF0YWJhc2UgdG8gY2FsY3VsYXRlIHRoZSBkYmZsYWdzLg0K Pj4gVGhpcyBpcyBzdGlsbCBkb25lIGJ1dCBsYXppbHkgZm9yIGVhY2ggYWNjZXNzZWQgZGF0YWJh c2Ugd2l0aCB0aGUgYXNzdW1wdGlvbg0KPj4gdGhhdCBhIHJlYWQgb25seSB0cmFuc2FjdGlvbiBy YXJlbHkgdXNlcyBhbGwgZGF0YWJhc2VzIG9mIHRoZSBlbnZpcm9ubWVudC4NCj4+DQo+PiBUaGUg bGF6eSBpbml0aWFsaXphdGlvbiBvZiB0aGUgZGJmbGFnIGhhcHBlbnMgaW4gdGhlIG1hY3JvIFRY Tl9EQklfRVhJU1Qgd2hpY2gNCj4+IGlzIGFsd2F5cyB1c2VkIHdoZW4gYSBkYXRhYmFzZSBoYW5k bGUgKGRiaSkgaXMgcGFzc2VkIHRvIGFuIGZ1bmN0aW9uLg0KPj4gVGhlIGZsYWdzIGFyZSB1cGRh dGVkIGluIG1kYl9zZXR1cF9kYl9pbmZvKCkgb25jZSBhIGRhdGFiYXNlIGlzIGFjY2VzcyB3aGlj aCBpcw0KPj4gbWFya2VkIGFzIHVudXNlZCAoREJfVU5VU0VEKS4NCj4+DQo+PiBzdGF0aWMgaW50 IG1kYl9zZXR1cF9kYl9pbmZvKE1EQl90eG4gKnR4biwgTURCX2RiaSBkYmkpIHsNCj4+ICAgICAg LyogU2V0dXAgZGIgaW5mbyAqLw0KPj4gICAgICB1aW50MTZfdCB4ID0gdHhuLT5tdF9lbnYtPm1l X2RiZmxhZ3NbZGJpXTsNCj4+ICAgICAgdHhuLT5tdF9kYnNbZGJpXS5tZF9mbGFncyA9IHggJiBQ RVJTSVNURU5UX0ZMQUdTOw0KPj4gICAgICB0eG4tPm10X2RiZmxhZ3NbZGJpXSA9ICh4ICYgTURC X1ZBTElEKSA/IERCX1ZBTElEfERCX1VTUlZBTElEfERCX1NUQUxFIDogMDsNCj4+ICAgICAgcmV0 dXJuICh0eG4tPm10X2RiZmxhZ3NbZGJpXSAmIHZhbGlkaXR5KTsNCj4+IH0NCj4+DQo+PiAgICAg IC8qKiBDaGVjayBcYiB0eG4gYW5kIFxiIGRiaSBhcmd1bWVudHMgdG8gYSBmdW5jdGlvbiBhbmQg aW5pdGlhbGl6ZSBkYiBpbmZvDQo+PiBpZiBuZWVkZWQgKi8NCj4+ICNkZWZpbmUgVFhOX0RCSV9F WElTVCh0eG4sIGRiaSwgdmFsaWRpdHkpIFwNCj4+ICAgICAgKCh0eG4pICYmIChkYmkjQzNDKHR4 biktPm10X251bWRicyAmJiAoKCh0eG4pLT5tdF9kYmZsYWdzW2RiaV0gJiAodmFsaWRpdHkpKQ0K Pj4gfHwgKCgodHhuKS0+bXRfZGJmbGFnc1tkYmldICYgREJfVU5VU0VEKSAmJiBtZGJfc2V0dXBf ZGJfaW5mbygodHhuKSwgKGRiaSksDQo+PiAodmFsaWRpdHkpKSkpKQ0KPj4NCj4+DQo+PiBUaGUg bmV4dCBpbXByb3ZlbWVudCBpcyBkb25lIGluIGFueSBmdW5jdGlvbiB3aGljaCBuZWVkcyB0byBs b29wIHRocm91Z2ggdGhlDQo+PiBkYXRhYmFzZXMgZm9yIGV4YW1wbGUgaW4gbWRiX2N1cnNvcnNf Y2xvc2UoKS4gQWdhaW4gdGhlIG1vcmUgZGF0YWJhc2VzIGluIHRoZQ0KPj4gZW52aXJvbm1lbnQg dGhlIGxvbmdlciB0aGUgZXhlY3V0aW9uIHRpbWUuDQo+PiBJdCBzaG91bGQgYmUgYmVzdCBpZiBs b29waW5nIG9ubHkgdGhyb3VnaCBkYmZsYWdzIGFuZCBzZWFyY2hpbmcgZm9yIHRob3NlDQo+PiBk YXRhYmFzZXMgd2hpY2ggYXJlIHVzZWQgKCFEQl9VTlVTRUQpLiBUaGlzIGNvdWxkIGJlIGRvbmUg Ynl0ZSB3aXNlIG9yIG1vcmUNCj4+IGVmZmljaWVudCBpbiA4LzQgYnl0ZSBzdGVwcyBjb21wYXJp bmcgd2l0aCBhbiBleHRlbmRlZCBtYXNrIERCX1VOVVNFRF9MT05HDQo+PiBpbnN0ZWFkIG9mIERC X1VOVVNFRC4gU28gd2UgY2FuIHNraXAgOCBvciA0ICgzMiBiaXQpIHVudXNlZCBkYXRhYmFzZXMg aW4gb25lDQo+PiBzdGVwIChzdGlsbCB3aXRoIHRoZSBhc3N1bXB0aW9uIHRoYXQgYSB0cmFuc2Fj dGlvbiByYXJlbHkgdXNlcyBhbGwgZGF0YWJhc2VzIG9mDQo+PiB0aGUgZW52aXJvbm1lbnQpLg0K Pj4NCj4+IFNvIHRoZSBsb29wIGxvb2tzIGFzIGZvbGxvd3MgYWx3YXlzIHN0YXJ0aW5nIGF0IHRo ZSBsb3dlciBpbmRleCB0byBhdm9pZA0KPj4gYWxpZ25tZW50IGlzc3VlcyB3aXRoIEFSTSBwcmlv ciB2Ni4NCj4+DQo+PiAjZGVmaW5lIERCX1VOVVNFRCAgICAgICAweDIwICAgICAgICAgICAgICAg IC8qKjwgREIgbm90IHVzZWQgaW4gdGhpcyB0eG4gKi8NCj4+DQo+PiAjaWZkZWYgTURCX1ZMMzIN Cj4+ICNkZWZpbmUgREJfVU5VU0VEX0xPTkcgIDB4MjAyMDIwMjAgICAgICAgICAgLyogREJfVU5V U0VEIGxvbmcgbWFzayBmb3IgZmFzdA0KPj4gdHJhY2tpbmcgKi8NCj4+ICNlbHNlDQo+PiAjZGVm aW5lIERCX1VOVVNFRF9MT05HICAweDIwMjAyMDIwMjAyMDIwMjAgIC8qIERCX1VOVVNFRCBsb25n IG1hc2sgZm9yIGZhc3QNCj4+IHRyYWNraW5nICovDQo+PiAjZW5kaWYNCj4+DQo+Pg0KPj4gI2lm ZGVmIE1EQl9WTDMyDQo+PiAjZGVmaW5lIE1EQl9XT1JEICAgIHVuc2lnbmVkIGludA0KPj4gI2Vs c2UNCj4+ICNkZWZpbmUgTURCX1dPUkQgICAgdW5zaWduZWQgbG9uZyBsb25nDQo+PiAjZW5kaWYN Cj4+DQo+Pg0KPj4NCj4+ICAgICAgTURCX2RiaSAlMz0gc3JjLT5tdF9udW1kYnM7DQo+PiAgICAg IE1EQl9kYmkgaSA9IDA7DQo+Pg0KPj4gICAgICB3aGlsZSAoMSkgew0KPj4gICAgICAgICAgdW5z aWduZWQgaW50IHVwcGVyID0gaSArIHNpemVvZihNREJfV09SRCk7DQo+PiAgICAgICAgICBpZiAo dXBwZXIgPCBuKSB7DQo+PiAgICAgICAgICAgICAgLy8gc2tpcCB1bnVzZWQNCj4+ICAgICAgICAg ICAgICBpZiAoKCooTURCX1dPUkQgKikodGRiZmxhZ3MgKyBpKSkgPT0gREJfVU5VU0VEX0xPTkcp IHsNCj4+ICAgICAgICAgICAgICAgICAgaSA9IHVwcGVyOw0KPj4gICAgICAgICAgICAgICAgICBj b250aW51ZTsNCj4+ICAgICAgICAgICAgICB9DQo+PiAgICAgICAgICB9DQo+PiAgICAgICAgICBl bHNlIHsNCj4+ICAgICAgICAgICAgICB1cHBlciA9IG47DQo+PiAgICAgICAgICB9DQo+Pg0KPj4g ICAgICAgICAgZm9yICg7IGkgPCB1cHBlcjsgaSsrKSB7DQo+PiAgICAgICAgICAgICAgLy8gYW55 IG90aGVyIGZpbHRlciBjcml0ZXJpYSBhcHByb3ByaWF0ZSB0byB0aGUgZnVuY3Rpb24NCj4+ICAg ICAgICAgICAgICAuLi4uDQo+PiAgICAgICAgICB9DQo+PiAgICAgICAgICBpZiAoaSA+PSBuKSB7 DQo+PiAgICAgICAgICAgICAgYnJlYWs7DQo+PiAgICAgICAgICB9DQo+PiAgICAgIH0NCj4+DQo+ Pg0KPj4NCj4NCj4NCj4tLSANCj4gICAtLSBIb3dhcmQgQ2h1DQo+ICAgQ1RPLCBTeW1hcyBDb3Jw LiAgICAgICAgICAgaHR0cDovL3d3dy5zeW1hcy5jb20NCj4gICBEaXJlY3RvciwgSGlnaGxhbmQg U3VuICAgICBodHRwOi8vaGlnaGxhbmRzdW4uY29tL2h5Yy8NCj4gICBDaGllZiBBcmNoaXRlY3Qs IE9wZW5MREFQICBodHRwOi8vd3d3Lm9wZW5sZGFwLm9yZy9wcm9qZWN0Lw0K

1 0

Re: (ITS#8430) Improved handling for large number of databases
by h.b.furuseth＠usit.uio.no 27 May '16

27 May '16

On 27. mai 2016 12:57, juerg.bircher(a)helmedica.com wrote: > mdb_dbi_open() is however not improved with the assumption that the database > handle (dbi) is cached in the application. So mdb_dbi_open() should happen only > once for each database during the life time of an application. It must check if the DB is open anyway, since LDMB breaks if the user has two DBIs to the same DB. Though maybe we could add an optimization flag to skip the search, for users who know they don't reopen open DBs. Other notes: LMDB and OpenLDAP mostly use tab-width = indentation = 4. (We should comment that somewhere.) You seem to use tab-width = 8. I think MDB_WORD intends to be mdb_size_t, but should be 'unsigned long': It's probably no optimization to use 'long long' when that is not a native integer type. But I haven't profiled that. The delay in picking up env->me_dbflags is a bit troubling, since it means creating a new txn is no longer quite atomic. I can't think of how it'll make a difference when DBIs are used correctly, but that's one thing people screw up. Maybe it'd be OK if renew0() just copies it into mt_dbs[].md_flags without the '&', and mdb_setup_db_info() fixes it up. -- Hallvard

1 0

Re: (ITS#8430) Improved handling for large number of databases
by hyc＠symas.com 27 May '16

27 May '16

juerg.bircher(a)helmedica.com wrote: > Full_Name: Juerg Bircher > Version: lmdb (master) > OS: MacOS / Linux > URL: ftp://ftp.openldap.org/incoming/Juerg_Bircher_160527-Improved-handling-for-… > Submission from: (NULL) (178.82.37.195) Thanks for the patch, but for a change of this size you also need to include an IP Rights notice, as documented on the Contributing guidelines. > > > There is a increased performance penalty the more databases are created within > the same environment. I was looking for a way the improve that by keeping the > simplicity of tracking databases within a list with direct access by index > (MDB_dbi). > mdb_dbi_open() is however not improved with the assumption that the database > handle (dbi) is cached in the application. So mdb_dbi_open() should happen only > once for each database during the life time of an application. > > > One issue is that mdb_txn_begin() (for read-only transactions) calloc the > sizeof(MDB_txn) + me_maxdbs * sizeof(MDB_db + 1). The plus 1 for the dbflags. > However it is sufficient only to malloc that size and clear the sizeof(MDB_txn) > > memset(txn, 0, sizeof(MDB_txn) > > After that the data beyond the MDB_txn is not initialized which is ok for the > moment. > > The next improvement happens in mdb_txn_renew0() where the dbflags are only set > to DB_UNUSED (a new flag) for each database currently opened in the > environment. > > memset(txn->mt_dbflags, DB_UNUSED, txn->mt_numdbs); > > The former code used to to loop through each database to calculate the dbflags. > This is still done but lazily for each accessed database with the assumption > that a read only transaction rarely uses all databases of the environment. > > The lazy initialization of the dbflag happens in the macro TXN_DBI_EXIST which > is always used when a database handle (dbi) is passed to an function. > The flags are updated in mdb_setup_db_info() once a database is access which is > marked as unused (DB_UNUSED). > > static int mdb_setup_db_info(MDB_txn *txn, MDB_dbi dbi) { > /* Setup db info */ > uint16_t x = txn->mt_env->me_dbflags[dbi]; > txn->mt_dbs[dbi].md_flags = x & PERSISTENT_FLAGS; > txn->mt_dbflags[dbi] = (x & MDB_VALID) ? DB_VALID|DB_USRVALID|DB_STALE : 0; > return (txn->mt_dbflags[dbi] & validity); > } > > /** Check \b txn and \b dbi arguments to a function and initialize db info > if needed */ > #define TXN_DBI_EXIST(txn, dbi, validity) \ > ((txn) && (dbi#C3C(txn)->mt_numdbs && (((txn)->mt_dbflags[dbi] & (validity)) > || (((txn)->mt_dbflags[dbi] & DB_UNUSED) && mdb_setup_db_info((txn), (dbi), > (validity))))) > > > The next improvement is done in any function which needs to loop through the > databases for example in mdb_cursors_close(). Again the more databases in the > environment the longer the execution time. > It should be best if looping only through dbflags and searching for those > databases which are used (!DB_UNUSED). This could be done byte wise or more > efficient in 8/4 byte steps comparing with an extended mask DB_UNUSED_LONG > instead of DB_UNUSED. So we can skip 8 or 4 (32 bit) unused databases in one > step (still with the assumption that a transaction rarely uses all databases of > the environment). > > So the loop looks as follows always starting at the lower index to avoid > alignment issues with ARM prior v6. > > #define DB_UNUSED 0x20 /**< DB not used in this txn */ > > #ifdef MDB_VL32 > #define DB_UNUSED_LONG 0x20202020 /* DB_UNUSED long mask for fast > tracking */ > #else > #define DB_UNUSED_LONG 0x2020202020202020 /* DB_UNUSED long mask for fast > tracking */ > #endif > > > #ifdef MDB_VL32 > #define MDB_WORD unsigned int > #else > #define MDB_WORD unsigned long long > #endif > > > > MDB_dbi %3= src->mt_numdbs; > MDB_dbi i = 0; > > while (1) { > unsigned int upper = i + sizeof(MDB_WORD); > if (upper < n) { > // skip unused > if ((*(MDB_WORD *)(tdbflags + i)) == DB_UNUSED_LONG) { > i = upper; > continue; > } > } > else { > upper = n; > } > > for (; i < upper; i++) { > // any other filter criteria appropriate to the function > .... > } > if (i >= n) { > break; > } > } > > > -- -- Howard Chu CTO, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/

1 0

(ITS#8431) LMDB: Access newly opened database from another transaction
by juerg.bircher＠helmedica.com 27 May '16

27 May '16

Full_Name: Jeerg Bircher Version: lmdb (master) OS: MacOS / Linux URL: ftp://ftp.openldap.org/incoming/Juerg_Bircher_160527-Access-newly-opened-da… Submission from: (NULL) (178.82.37.195) A transaction tracks newly opened databases and if the transaction is committed the newly opened databases are propagated to the list of open databases of the environment. However if the read only transaction is aborted the databases are not propagated. If database handles (MDB_dbi) are cached in the application to avoid calling mdb_dbi_open() there might be the situation of two threads running a read only transaction concurrently. First threads opens the database and commits the read-only transaction. The database is added to the list of open databases in the environment. The returned dbi is globally cached in the application. The second thread also wants to access the same database and finds the database handle in the global application cache. The database handle however is not valid as the transaction only uses a snapshot of open databases. So the second thread gets an EINVAL when using that database handle. This should not happen as the database is open and added to the environment. The following should fix this issue by updating the mt_numdbs and marking the delta with DB_UNUSED static int mdb_update_db_info(MDB_txn *txn, MDB_dbi dbi) { /* propagate newly opened db from env to current txn. Mark them as unused */ if (txn->mt_numdbs < txn->mt_env->me_numdbs) { memset(txn->mt_dbflags + txn->mt_numdbs, DB_UNUSED, txn->mt_env->me_numdbs - txn->mt_numdbs); } txn->mt_numdbs = txn->mt_env->me_numdbs; return dbi < txn->mt_numdbs; } /** Check \b txn and \b dbi arguments to a function and initialize db info if needed */ #define TXN_DBI_EXIST(txn, dbi, validity) \ ((txn) && ((dbi)<(txn)->mt_numdbs || mdb_update_db_info((txn), (dbi))) && (((txn)->mt_dbflags[dbi] & (validity)) || (((txn)->mt_dbflags[dbi] & DB_UNUSED) && mdb_setup_db_info((txn), (dbi), (validity))))) IMPORTANT depends ITS#8430 See also mailing list openldap-technical thread: Improved handling for large number of databases / Access newly opened database from another transaction

1 0

(ITS#8430) Improved handling for large number of databases
by juerg.bircher＠helmedica.com 27 May '16

27 May '16

Full_Name: Juerg Bircher Version: lmdb (master) OS: MacOS / Linux URL: ftp://ftp.openldap.org/incoming/Juerg_Bircher_160527-Improved-handling-for-… Submission from: (NULL) (178.82.37.195) There is a increased performance penalty the more databases are created within the same environment. I was looking for a way the improve that by keeping the simplicity of tracking databases within a list with direct access by index (MDB_dbi). mdb_dbi_open() is however not improved with the assumption that the database handle (dbi) is cached in the application. So mdb_dbi_open() should happen only once for each database during the life time of an application. One issue is that mdb_txn_begin() (for read-only transactions) calloc the sizeof(MDB_txn) + me_maxdbs * sizeof(MDB_db + 1). The plus 1 for the dbflags. However it is sufficient only to malloc that size and clear the sizeof(MDB_txn) memset(txn, 0, sizeof(MDB_txn) After that the data beyond the MDB_txn is not initialized which is ok for the moment. The next improvement happens in mdb_txn_renew0() where the dbflags are only set to DB_UNUSED (a new flag) for each database currently opened in the environment. memset(txn->mt_dbflags, DB_UNUSED, txn->mt_numdbs); The former code used to to loop through each database to calculate the dbflags. This is still done but lazily for each accessed database with the assumption that a read only transaction rarely uses all databases of the environment. The lazy initialization of the dbflag happens in the macro TXN_DBI_EXIST which is always used when a database handle (dbi) is passed to an function. The flags are updated in mdb_setup_db_info() once a database is access which is marked as unused (DB_UNUSED). static int mdb_setup_db_info(MDB_txn *txn, MDB_dbi dbi) { /* Setup db info */ uint16_t x = txn->mt_env->me_dbflags[dbi]; txn->mt_dbs[dbi].md_flags = x & PERSISTENT_FLAGS; txn->mt_dbflags[dbi] = (x & MDB_VALID) ? DB_VALID|DB_USRVALID|DB_STALE : 0; return (txn->mt_dbflags[dbi] & validity); } /** Check \b txn and \b dbi arguments to a function and initialize db info if needed */ #define TXN_DBI_EXIST(txn, dbi, validity) \ ((txn) && (dbi#C3C(txn)->mt_numdbs && (((txn)->mt_dbflags[dbi] & (validity)) || (((txn)->mt_dbflags[dbi] & DB_UNUSED) && mdb_setup_db_info((txn), (dbi), (validity))))) The next improvement is done in any function which needs to loop through the databases for example in mdb_cursors_close(). Again the more databases in the environment the longer the execution time. It should be best if looping only through dbflags and searching for those databases which are used (!DB_UNUSED). This could be done byte wise or more efficient in 8/4 byte steps comparing with an extended mask DB_UNUSED_LONG instead of DB_UNUSED. So we can skip 8 or 4 (32 bit) unused databases in one step (still with the assumption that a transaction rarely uses all databases of the environment). So the loop looks as follows always starting at the lower index to avoid alignment issues with ARM prior v6. #define DB_UNUSED 0x20 /**< DB not used in this txn */ #ifdef MDB_VL32 #define DB_UNUSED_LONG 0x20202020 /* DB_UNUSED long mask for fast tracking */ #else #define DB_UNUSED_LONG 0x2020202020202020 /* DB_UNUSED long mask for fast tracking */ #endif #ifdef MDB_VL32 #define MDB_WORD unsigned int #else #define MDB_WORD unsigned long long #endif MDB_dbi %3= src->mt_numdbs; MDB_dbi i = 0; while (1) { unsigned int upper = i + sizeof(MDB_WORD); if (upper < n) { // skip unused if ((*(MDB_WORD *)(tdbflags + i)) == DB_UNUSED_LONG) { i = upper; continue; } } else { upper = n; } for (; i < upper; i++) { // any other filter criteria appropriate to the function .... } if (i >= n) { break; } }

1 0

Re: (ITS#8427) Incorrect value of tls_reqcert in syncrepl
by mkp37215＠gmail.com 20 May '16

20 May '16

Howard, thanks for the reply. I just noticed a small error in what I wrote, the corrected fragment should be: "This was because sb->sb_tls_do_init was FALSE and bindconf_tls_set(sb, ld) was not called." I also would like to add that my patch changes the semantics of bindconf_tls_set, in regard of how TLS context is set, and that this is deliberate. I think that previous semantics was unclear and bug-prone, and that the new one is not only more straightforward, but also matches better the way bindconf_tls_set is used. As a result both bindconf_tls_set code and the code around its invocations is simplified. However, I was focused on its usage in slap_client_connect (because this is what was causing me problems), and I did not put much attention into other three places where bindconf_tls_set is called. All of those code fragments were basically identical, so I modified them the same way, but I think someone should review these modifications to see if they make sense. I originally intended to limit the impact of my patch to slap_client_connect, and to keep the changes inside config.c file. However, this resulted in making bad code worse, even less clear and manageable.

1 0

Re: (ITS#8427) Incorrect value of tls_reqcert in syncrepl
by hyc＠symas.com 20 May '16

20 May '16

mkp37215(a)gmail.com wrote: > The root cause of the problem: Thanks for the analysis, will look into it soon. > On syncrepl retry, code in slap_client_connect > (servers/slapd/config.c) initialized ld (LDAP) object with default > values, rather than values saved in sb (slap_bindconf). This was > because sb->sb_tls_do_init was true and bindconf_tls_set(sb, ld) was > not called. > This issue is not related to any specific TLS library. All syncrepl > retries are executed with wrong TLS settings in ld->ld_options, > including ldo_tls_require_cert, ldo_tls_keyfile, ldo_tls_certfile, > ldo_tls_cacertfile, and other (ldo_tls_require_cert has default value > 2 (demand), while string fields have nulls). However, the bug is > usually masked, because sb->sb_tls_ctx contains TLS context correctly > initialized during initial connection attempt, and this context is > assigned to ld as LDAP_OPT_X_TLS_CTX, and then used by the TLS > library, while incorrect values in ld->ld_options are ignored in most > places. But the host-checking code in ldap_int_tls_start > (libraries/libldap/tls2.c) uses ld->ld_options and triggers the bug. That description makes sense, OK. > Explanation of the patch: > The patch modifies function bindconf_tls_set that copies values from > slap_bindconf to LDAP object. Code that calls bindconf_tls_set is also > modified. > 1. If slap_bindconf object has TLS context, assign it to LDAP object. > 2. if slap_bindconf does not have a TLS context, create new context > and assign it it to both slap_bindconf and LDAP objects. > 3. Removed context replacement functionality and related conditions > (newctx). The intent of this functionality was unclear, but did not > appear to be compatible with how binconf_tls_set was used. > 4. The result of the above code changes is that TLS context is always > present and correctly assigned, while previously it sometimes was, and > sometimes wasn't. > 5. Simplified calls of bindconf_tls_set in the following files: > servers/slapd/config.c > servers/slapd/back-ldap/bind.c > servers/slapd/back-meta/conn.c > servers/slapd/back-asyncmeta/conn.c > > Things to do: > I tested the modified slap_client_connect (servers/slapd/config.c), > but the modifications to files in back-{ldap,meta,asyncmeta} were done > without proper code and functionality analysis, and without any > testing. I am hoping for a review from someone knowledgeable. > > Thank you > > > > -- -- Howard Chu CTO, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

2007

2006

openldap-bugs