Matthieu GUEGAN wrote:
> On Fri, Feb 15, 2019 at 3:56 PM Howard Chu <hyc(a)symas.com> wrote:
>>
>> mguegan(a)virtua.ch wrote:
>>> Full_Name: Matthieu Guegan
>>> Version: current
>>> OS: SmartOS
>>> URL:
>>> Submission from: (NULL) (109.190.148.77)
>>>
>>>
>>> Hi,
>>>
>>> In order to compile knot[1] on SmartOS, I have done a series of little patches.
>>> One of them is linked to the LMDB project, which redirects me here.
>>>
>>> The idea is to tell SmartOS (and I think the SunOS family) to make use of
>>> posix_madvise(3), instead of madvise(3) as implementation on this OS[2] is
>>> different than, for example, the Linux[3] one.
>>>
>>> So, I kindly ask you to take a look at the merge request of the knot project[4],
>>> in particular the `src/contrib/lmdb/mdb.c` patch (that's just a ifdef
>>> modification) : would it be possible to apply this patch directly in this
>>> repository ?
>>
>> I see no functional difference between your references [2] and [3].
>> What is the benefit of this patch?
>
> Well, the Solaris/SunOS's madvise(3) is defined with an old `caddr_t`
> type whereas Linux (and *BSD OSes too) use a more modern approach with
> `void *`.
> On the `knot` project, which embed the lmdb part of openldap,
> compilation failed when using madvise(3) on SmartOS, so I did a
> replacement with posix_madvise(3) which uses the `void *` argument
> too.
> I don't know what's the best way to handle this case, that's why I ask here.
A caddr_t is just a typedef of "char *". It should always be legal to
pass a void * to such a parameter. What is your compile error message?
--
-- Howard Chu
CTO, Symas Corp. http://www.symas.com
Director, Highland Sun http://highlandsun.com/hyc/
Chief Architect, OpenLDAP http://www.openldap.org/project/
On Fri, Feb 15, 2019 at 3:56 PM Howard Chu <hyc(a)symas.com> wrote:
>
> mguegan(a)virtua.ch wrote:
> > Full_Name: Matthieu Guegan
> > Version: current
> > OS: SmartOS
> > URL:
> > Submission from: (NULL) (109.190.148.77)
> >
> >
> > Hi,
> >
> > In order to compile knot[1] on SmartOS, I have done a series of little patches.
> > One of them is linked to the LMDB project, which redirects me here.
> >
> > The idea is to tell SmartOS (and I think the SunOS family) to make use of
> > posix_madvise(3), instead of madvise(3) as implementation on this OS[2] is
> > different than, for example, the Linux[3] one.
> >
> > So, I kindly ask you to take a look at the merge request of the knot project[4],
> > in particular the `src/contrib/lmdb/mdb.c` patch (that's just a ifdef
> > modification) : would it be possible to apply this patch directly in this
> > repository ?
>
> I see no functional difference between your references [2] and [3].
> What is the benefit of this patch?
Well, the Solaris/SunOS's madvise(3) is defined with an old `caddr_t`
type whereas Linux (and *BSD OSes too) use a more modern approach with
`void *`.
On the `knot` project, which embed the lmdb part of openldap,
compilation failed when using madvise(3) on SmartOS, so I did a
replacement with posix_madvise(3) which uses the `void *` argument
too.
I don't know what's the best way to handle this case, that's why I ask here.
> >
> > Thanks
> >
> >
> > [1] https://www.knot-dns.cz/
> > [2] https://docs.oracle.com/cd/E36784_01/html/E36874/madvise-3c.html
> > [3] http://man7.org/linux/man-pages/man2/madvise.2.html
> > [4] https://gitlab.labs.nic.cz/knot/knot-dns/merge_requests/979/diffs?commit_id…
> >
> >
>
>
> --
> -- Howard Chu
> CTO, Symas Corp. http://www.symas.com
> Director, Highland Sun http://highlandsun.com/hyc/
> Chief Architect, OpenLDAP http://www.openldap.org/project/
mguegan(a)virtua.ch wrote:
> Full_Name: Matthieu Guegan
> Version: current
> OS: SmartOS
> URL:
> Submission from: (NULL) (109.190.148.77)
>
>
> Hi,
>
> In order to compile knot[1] on SmartOS, I have done a series of little patches.
> One of them is linked to the LMDB project, which redirects me here.
>
> The idea is to tell SmartOS (and I think the SunOS family) to make use of
> posix_madvise(3), instead of madvise(3) as implementation on this OS[2] is
> different than, for example, the Linux[3] one.
>
> So, I kindly ask you to take a look at the merge request of the knot project[4],
> in particular the `src/contrib/lmdb/mdb.c` patch (that's just a ifdef
> modification) : would it be possible to apply this patch directly in this
> repository ?
I see no functional difference between your references [2] and [3].
What is the benefit of this patch?
>
> Thanks
>
>
> [1] https://www.knot-dns.cz/
> [2] https://docs.oracle.com/cd/E36784_01/html/E36874/madvise-3c.html
> [3] http://man7.org/linux/man-pages/man2/madvise.2.html
> [4] https://gitlab.labs.nic.cz/knot/knot-dns/merge_requests/979/diffs?commit_id…
>
>
--
-- Howard Chu
CTO, Symas Corp. http://www.symas.com
Director, Highland Sun http://highlandsun.com/hyc/
Chief Architect, OpenLDAP http://www.openldap.org/project/
Full_Name: Howard Chu
Version: 2.5
OS:
URL: ftp://ftp.openldap.org/incoming/
Submission from: (NULL) (80.233.42.124)
Submitted by: hyc
IDLs used for index slots are hardcoded to a max of 65536 elements in the DB and
twice that in memory. Sites with larger DBs tend to need larger IDLs; if they're
running with sufficient memory it ought to be possible to configure larger IDLs.
Full_Name: Matthieu Guegan
Version: current
OS: SmartOS
URL:
Submission from: (NULL) (109.190.148.77)
Hi,
In order to compile knot[1] on SmartOS, I have done a series of little patches.
One of them is linked to the LMDB project, which redirects me here.
The idea is to tell SmartOS (and I think the SunOS family) to make use of
posix_madvise(3), instead of madvise(3) as implementation on this OS[2] is
different than, for example, the Linux[3] one.
So, I kindly ask you to take a look at the merge request of the knot project[4],
in particular the `src/contrib/lmdb/mdb.c` patch (that's just a ifdef
modification) : would it be possible to apply this patch directly in this
repository ?
Thanks
[1] https://www.knot-dns.cz/
[2] https://docs.oracle.com/cd/E36784_01/html/E36874/madvise-3c.html
[3] http://man7.org/linux/man-pages/man2/madvise.2.html
[4] https://gitlab.labs.nic.cz/knot/knot-dns/merge_requests/979/diffs?commit_id…
A new patch with added comments has been added at
ftp://ftp.openldap.org/incoming/nadezhda-ivanova-190214.patch
More details about this issue:
The customer reported that when they simulated a "network unreachable" error, SLAPD exited with the following assertion error:
#0 0x00007fa1d19e4885 in raise () from /lib64/libc.so.6
#1 0x00007fa1d19e5e61 in abort () from /lib64/libc.so.6
#2 0x00007fa1d19dd740 in __assert_fail () from /lib64/libc.so.6
#3 0x0000000000449c17 in slap_send_ldap_result (op=0x1870000, rs=0x7fa17c742930) at result.c:830
#4 0x00000000004b6a53 in meta_back_search (op=0x1870000, rs=0x7fa17c742930) at search.c:1183
#5 0x000000000043aef1 in fe_op_search (op=0x1870000, rs=0x7fa17c742930) at search.c:402
#6 0x00000000004a1a62 in overlay_op_walk (op=0x1870000, rs=0x7fa17c742930, which=op_search, oi=0x88583c0, on=0x0) at backover.c:677
#7 0x00000000004a24dc in over_op_func (op=0x1870000, rs=0x8, which=op_add) at backover.c:730
#8 0x000000000043b73c in do_search (op=0x1870000, rs=0x7fa17c742930) at search.c:247
#9 0x00000000004382af in connection_operation (ctx=0x7fa17c742b30, arg_v=<optimized out>) at connection.c:1316
#10 0x0000000000438edb in connection_read_thread (ctx=0x7fa17c742b30, argv=<optimized out>) at connection.c:1452
#11 0x00000000004f06a0 in ldap_int_thread_pool_wrapper (xpool=<optimized out>) at tpool.c:696
#12 0x00007fa1d1d35806 in start_thread () from /lib64/libpthread.so.0
#13 0x00007fa1d1a9065d in clone () from /lib64/libc.so.6
#14 0x0000000000000000 in ?? ()
This happened because meta used to directly return the ldap_sasl_bind return value to the client, and in this case it is not an LDAP error.
We fixed the issue by mapping it to an LDAP error.
Nadezhda Ivanova
Software Engineer
Symas Corporation http://www.symas.com
Sharma, Ramakant 2. (Nokia - IN/Bangalore) wrote:
> Hi Howard,
>
> Please provide your valuable inputs for the below patch.
It is a little strange that ldap_set_option() takes a string but ldap_get_option()
returns a binary structure. get/set should be totally symmetric. It would be
simplest to just save a copy of the string given to set and return the string in get.
In libldap/os-ip.c, you should not be using ldap_get_option() to retrieve the values.
You're operating inside libldap, you can access the structure directly. In particular,
it's a waste to use ldap_get_option here and create a dynamically allocated copy of the
address struct. Look at how the existing code works. Notice that it just uses
ld->ld_options directly.
This is rule #1 in software development - when extending existing code, your new code
should do things the same way existing code does. Corollaries to this rule are:
don't patch code without reading it first, and don't patch code without studying
its complete context.
>
> Also let us know how to proceed further.
>
> BR,
> Ramakant Sharma
>
> -----Original Message-----
> From: Sharma, Ramakant 2. (Nokia - IN/Bangalore)
> Sent: Monday, January 28, 2019 10:10 AM
> To: Howard Chu <hyc(a)symas.com>; openldap-its(a)OpenLDAP.org
> Cc: Singam, Sudhir (Nokia - IN/Bangalore) <sudhir.singam(a)nokia.com>
> Subject: RE: (ITS#8847) New LDAP URL syntax to support binding to specific IP address at client side
>
> Hi Howard,
>
> I have uploaded the new patch which addresses the previous comments.
>
> Kindly check and provide your valuable comments .
>
> ftp://ftp.openldap.org/incoming/ramakant-sharma-190121.patch
>
> BR,
> Ramakant Sharma
>
--
-- Howard Chu
CTO, Symas Corp. http://www.symas.com
Director, Highland Sun http://highlandsun.com/hyc/
Chief Architect, OpenLDAP http://www.openldap.org/project/
kriszyp(a)gmail.com wrote:
> Full_Name: Kristopher William Zyp
> Version: LMDB 0.9.23
> OS: Windows Server 2012 R2, Windows 10
> URL: https://github.com/kriszyp/node-lmdb/commit/6df903907f5516320e9a8afce45bd32…
> Submission from: (NULL) (71.199.6.148)
>
Thanks for the report and patch, added to mdb.RE/0.9
> Calling mdb_env_set_mapsize to increase the map size, when a DB is also in use
> by other processes, when MDB_WRITEMAP is enabled (and the db file has been
> opened with PAGE_READWRITE access), on Windows, will occasionally (seems like
> about 1/100 attempts fails) produce an error "The requested operation cannot be
> performed on a file with a user-mapped section open", or segfaults. The error
> occurs in the SetFilePointer (or SetEndOfFile) call in mdb_env_map that is
> performed to increase the allocated file size to the map size, prior to
> CreateFileMapping.
>
> As it turns out this is pretty easy to solve, because manually expanding the
> file size is not necessary when calling CreateFileMapping with PAGE_READWRITE
> access, as Windows will automatically expand the file size for us, when opened
> with the page write access enabled. Of course, this means all processes must be
> consistent in use of MDB_WRITEMAP, but the documentation already makes this
> explicit and clear.
>
> I believe this can be fixed by simply adding a check for MDB_WRITEMAP in the if
> statement that calls SetFilePointer:
>
> if (!(flags & MDB_WRITEMAP) && (SetFilePointer(env->me_fd, sizelo, &sizehi, 0)
> != (DWORD)sizelo
> || !SetEndOfFile(env->me_fd)
> || SetFilePointer(env->me_fd, 0, NULL, 0) != 0))
> return ErrCode();
>
> The attached URL has the change as a patch/diff as applied to our node package.
>
> I am certainly happy to just keep this change on our own branches. There may be
> nuances of this that I might not be aware of, but it seems to be working great
> for us and I have tested this with MDB_WRITEMAP enabled and disabled. So I
> thought I would offer/suggest this change, as it seems like it is
> straightforward change to improve stability. 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/
Full_Name: Kristopher William Zyp
Version: LMDB 0.9.23
OS: Windows Server 2012 R2, Windows 10
URL: https://github.com/kriszyp/node-lmdb/commit/6df903907f5516320e9a8afce45bd32…
Submission from: (NULL) (71.199.6.148)
Calling mdb_env_set_mapsize to increase the map size, when a DB is also in use
by other processes, when MDB_WRITEMAP is enabled (and the db file has been
opened with PAGE_READWRITE access), on Windows, will occasionally (seems like
about 1/100 attempts fails) produce an error "The requested operation cannot be
performed on a file with a user-mapped section open", or segfaults. The error
occurs in the SetFilePointer (or SetEndOfFile) call in mdb_env_map that is
performed to increase the allocated file size to the map size, prior to
CreateFileMapping.
As it turns out this is pretty easy to solve, because manually expanding the
file size is not necessary when calling CreateFileMapping with PAGE_READWRITE
access, as Windows will automatically expand the file size for us, when opened
with the page write access enabled. Of course, this means all processes must be
consistent in use of MDB_WRITEMAP, but the documentation already makes this
explicit and clear.
I believe this can be fixed by simply adding a check for MDB_WRITEMAP in the if
statement that calls SetFilePointer:
if (!(flags & MDB_WRITEMAP) && (SetFilePointer(env->me_fd, sizelo, &sizehi, 0)
!= (DWORD)sizelo
|| !SetEndOfFile(env->me_fd)
|| SetFilePointer(env->me_fd, 0, NULL, 0) != 0))
return ErrCode();
The attached URL has the change as a patch/diff as applied to our node package.
I am certainly happy to just keep this change on our own branches. There may be
nuances of this that I might not be aware of, but it seems to be working great
for us and I have tested this with MDB_WRITEMAP enabled and disabled. So I
thought I would offer/suggest this change, as it seems like it is
straightforward change to improve stability. Thank you!
--_FEED8D1F-4CE2-4DE8-A6B1-1703AFFCC4FC_
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset="utf-8"
To follow up to this, after switching to the stable release in node-ldmb, a=
nd running on our servers for several days, I can confirm this definitely d=
oes fix our issues (and ticket can be closed). I=E2=80=99ve submitted a PR =
for getting the correct stable version in node-lmdb, to ensure in this does=
n=E2=80=99t happen for other NodeJS users: https://github.com/Venemo/node-l=
mdb/pull/141=20
I did find a minor issue with setting up the file mapping in the release ve=
rsion, but I will file that separately. And if the NTDLL mapping isn=E2=80=
=99t intended to be merged into a release branch, sounds like there shouldn=
=E2=80=99t be any issues for Windows users that are using LMDB releases.
And to be clear, I am a happy windows user, sorting out versions hardly den=
ts my enthusiasm for LMDB. And thank you again for the help.
Thanks,
Kris
--_FEED8D1F-4CE2-4DE8-A6B1-1703AFFCC4FC_
Content-Transfer-Encoding: quoted-printable
Content-Type: text/html; charset="utf-8"
<html xmlns:o=3D"urn:schemas-microsoft-com:office:office" xmlns:w=3D"urn:sc=
hemas-microsoft-com:office:word" xmlns:m=3D"http://schemas.microsoft.com/of=
fice/2004/12/omml" xmlns=3D"http://www.w3.org/TR/REC-html40"><head><meta ht=
tp-equiv=3DContent-Type content=3D"text/html; charset=3Dutf-8"><meta name=
=3DGenerator content=3D"Microsoft Word 15 (filtered medium)"><style><!--
/* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0cm;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:#954F72;
text-decoration:underline;}
.MsoChpDefault
{mso-style-type:export-only;}
@page WordSection1
{size:612.0pt 792.0pt;
margin:72.0pt 72.0pt 72.0pt 72.0pt;}
div.WordSection1
{page:WordSection1;}
--></style></head><body lang=3DEN-CA link=3Dblue vlink=3D"#954F72"><div cla=
ss=3DWordSection1><p class=3DMsoNormal>To follow up to this, after switchin=
g to the stable release in node-ldmb, and running on our servers for severa=
l days, I can confirm this definitely does fix our issues (and ticket can b=
e closed). I=E2=80=99ve submitted a PR for getting the correct stable versi=
on in node-lmdb, to ensure in this doesn=E2=80=99t happen for other NodeJS =
users: <a href=3D"https://github.com/Venemo/node-lmdb/pull/141">https://git=hub.com/Venemo/node-lmdb/pull/141</a> </p><p class=3DMsoNormal><o:p> <=
/o:p></p><p class=3DMsoNormal>I did find a minor issue with setting up the =
file mapping in the release version, but I will file that separately. And i=
f the NTDLL mapping isn=E2=80=99t intended to be merged into a release bran=
ch, sounds like there shouldn=E2=80=99t be any issues for Windows users tha=
t are using LMDB releases.</p><p class=3DMsoNormal><o:p> </o:p></p><p =
class=3DMsoNormal>And to be clear, I am a happy windows user, sorting out v=
ersions hardly dents my enthusiasm for LMDB. And thank you again for the he=
lp.<o:p></o:p></p><p class=3DMsoNormal><o:p> </o:p></p><p class=3DMsoN=
ormal>Thanks,<br>Kris<o:p></o:p></p></div></body></html>=
--_FEED8D1F-4CE2-4DE8-A6B1-1703AFFCC4FC_--