[389-devel] Re: Logging future direction and ideas.
> On 17 May 2019, at 00:16, Ludwig Krispenz wrote: > > > On 05/16/2019 02:56 AM, William Brown wrote: >> Replying to both Simon and Ludwig in the one mail ... >> >> To just comment to one point from Ludwig: >> >>> This may sound too negative, I agree that logging deserves attention and >>> improvement, but we need to agree on what we want to achieve. >> I don't think this is negative, at all and I think it's exactly the kind of >> feedback I wanted. :) So thank you! >> >> >>> Hi William, >>> I am more than interested! >>> >>> I'd like to learn it one day anyway. >>> So if there will be no objections and we'll go forward now, >>> I think, it is important to agree on some points of decreasing a bus factor: >>> >>> - As you said, it should have very detailed comments on all of the >>> language features and technics you'll use. >>> - I think it will be nice to have an additional documentation which will >>> describe the basic setup for the development you use. All the toold you >>> need to develop, compile, test and debug. >>> - Also, some nice links for the basic tutorials on Rust types, concepts, >>> etc. >>> - I think, we should have detailed unit tests. It will help to >>> understand the code better and we will have less bugs (hopefully). >> So I'm writing a document for the wiki to cover this *and* rust supports >> tests as part of the system - that will be all be included. Is that >> acceptable? >> >>> And the final and big point I wanted to mention: >>> >>> - We should be prepared for a slow review process because we have quite >>> limited >>> recources in the team and a lot of work should be done (WebUI still has to >>> be refactored to React, >>> and it is only a small piece of the workload). >>> Also, I think, it makes sense to have the smallest Rust PRs that can be >>> put together as an independent unit. >> That's totally reasonable. I'll try to keep it to small parts and will build >> up as we go. See below for ideas on how I'll work toward this. >> >>> But everything is just my opinion and I don't know what others think and >>> if everyone is prepared to join it. I am feeling excited though :) >>> >>> Thanks, >>> Simon >>> >>> P.S. check the contribution guide please. Espesially a part about >>> 'rebase-force-push'. I think it is nice not to force push during >>> the review process (and rebase-squash only after you got an ACK). >> Yep, I saw this change. I'll keep it in mind :) >> >> >>> On 15 May 2019, at 19:37, Ludwig wrote: >>> >>> Hi William, >>> >>> this is a good starting point to discuss and decide how to move forward >>> with logging (although it looks like you are already some steps ahead). >>> >>> I have no strong ties to existing logging format and if we do it in rust or >>> not I don't care, but I do not want to use existing functionality for the >>> sake af a new unified approach. >>> >>> First we need to define what is the purpose and usage of our logs, what do >>> we want to keep or extend. I see these main area: >>> >>> - statistics >>> >>> - auditing (authentications, modifications) >>> >>> . debugging (for my usage the most important aspect) >>> >>> Next: what are the real problems >>> >>> - performance: yes, it would be nice to have less logging impact on >>> performance, but I haven't seen real complaints and doing it differently >>> does not guarantee better performance, we have debug levels (like tracing) >>> which are not usable because they slow down everything, I do not think this >>> will be resolved by just replacing the library >>> >>> - information: we continue to add information to the logs, and there are >>> still open requests >>> >>> >>> So: If I look at your suggestions I do like some and have concerns with >>> others, and I am not sure if the priorities are right. >>> >>> You list a couple of tickets related to logging, but forgot others, eg: >>> >>> https://pagure.io/389-ds-base/issue/47822 - improve logging of ACL decisions >>> >>> https://pagure.io/389-ds-base/issue/48680 - enable logging for 3rd party >>> libs >>> >>> https://pagure.io/389-ds-base/issue/50002 - improve password policy logging >>> >>> https://pagure.io/389-ds-base/issue/50187 - improve replication logging >> These are all things that you you are correct, that I missed. >> >> So I want to focus on your point about what we want from logs, IE stats, >> auditing, debugging. I think this is a great summary of what we want, but to >> make it more detailed: >> >> * Stats about operations from start to finish >> * Auditing of client operations >> * Debugging of client operations >> * Debugging of internal server machinery and state >> * Reporting of errors outside of the normal operation system. >> >> Today we current have: >> >> access - auditing of operations at a highlevel >> audit - auditing of modifications and writes >> error - debugging of internal server machinery (we tend not to enable the >> levels of debugging
[389-devel] Re: Advice on a memory issue
> On 16 May 2019, at 17:18, thierry bordaz wrote: > > Hi William, > > It looks to me that attr_syntax_create overwrite the allocated asi with one > it allocates itself based on provided params. > In short I think attr_syntax_creates allocates for you the syntaxinfo, you do > not need to provide one. > Ah how did I miss this!!! I was staring attr_syntax_create for most of the morning yesterday! Anyway, thank you, I have fixed that up - back to writing tests now ... > best regards > thierry > On 5/16/19 8:17 AM, William Brown wrote: >> https://pagure.io/389-ds-base/pull-request/50379 >> >> >> This code is not yet ready to be merged. I'm currently having a problem with >> freeing the attrsyntaxinfo struct as part of the test. >> >> If the code is as is I get: >> >> >> = >> ==98363==ERROR: LeakSanitizer: detected memory leaks >> >> Direct leak of 160 byte(s) in 1 object(s) allocated from: >> #0 0x7fbc40e28538 in calloc (/usr/lib64/libasan.so.5+0xec538) >> #1 0x7fbc40a34be6 in slapi_ch_calloc >> /home/william/development/389ds/ds/ldap/servers/slapd/ch_malloc.c:175 >> #2 0x40499c in attr_syntax_add_from_name >> /home/william/development/389ds/ds/test/libslapd/schema/filter_validate.c:25 >> #3 0x404b22 in test_libslapd_schema_filter_validate_simple >> /home/william/development/389ds/ds/test/libslapd/schema/filter_validate.c:56 >> #4 0x7fbc40d340d8 (/usr/lib64/libcmocka.so.0+0x50d8) >> >> Objects leaked above: >> 0x60e00d60 (160 bytes) >> >> Direct leak of 160 byte(s) in 1 object(s) allocated from: >> #0 0x7fbc40e28538 in calloc (/usr/lib64/libasan.so.5+0xec538) >> #1 0x7fbc40a34be6 in slapi_ch_calloc >> /home/william/development/389ds/ds/ldap/servers/slapd/ch_malloc.c:175 >> #2 0x40499c in attr_syntax_add_from_name >> /home/william/development/389ds/ds/test/libslapd/schema/filter_validate.c:25 >> #3 0x404b0f in test_libslapd_schema_filter_validate_simple >> /home/william/development/389ds/ds/test/libslapd/schema/filter_validate.c:55 >> #4 0x7fbc40d340d8 (/usr/lib64/libcmocka.so.0+0x50d8) >> >> Objects leaked above: >> 0x60e00ba0 (160 bytes) >> >> SUMMARY: AddressSanitizer: 320 byte(s) leaked in 2 allocation(s). >> >> However, if I free *a and *b, with attr_syntax_free, or slapi_ch_free, I get >> a double free error. The size of 160 bytes correlates to the sizeof(struct >> attrsyntaxinfo) but looking in gdb during attr_syntax_delete, the >> attr_syntax_free is called on asi as provided. >> >> So I'm not 100% sure what's going wrong here, but I'm not thoroughly >> experienced in this part of the code, so feedback would be really helpful >> about this resource issue. >> >> Thanks! >> >> — >> Sincerely, >> >> William Brown >> >> Senior Software Engineer, 389 Directory Server >> SUSE Labs >> ___ >> 389-devel mailing list -- >> 389-devel@lists.fedoraproject.org >> >> To unsubscribe send an email to >> 389-devel-le...@lists.fedoraproject.org >> >> Fedora Code of Conduct: >> https://getfedora.org/code-of-conduct.html >> >> List Guidelines: >> https://fedoraproject.org/wiki/Mailing_list_guidelines >> >> List Archives: >> https://lists.fedoraproject.org/archives/list/389-devel@lists.fedoraproject.org > — Sincerely, William Brown Senior Software Engineer, 389 Directory Server SUSE Labs ___ 389-devel mailing list -- 389-devel@lists.fedoraproject.org To unsubscribe send an email to 389-devel-le...@lists.fedoraproject.org Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/389-devel@lists.fedoraproject.org
[389-devel] Re: Logging future direction and ideas.
On 05/16/2019 02:56 AM, William Brown wrote: Replying to both Simon and Ludwig in the one mail ... To just comment to one point from Ludwig: This may sound too negative, I agree that logging deserves attention and improvement, but we need to agree on what we want to achieve. I don't think this is negative, at all and I think it's exactly the kind of feedback I wanted. :) So thank you! Hi William, I am more than interested! I'd like to learn it one day anyway. So if there will be no objections and we'll go forward now, I think, it is important to agree on some points of decreasing a bus factor: - As you said, it should have very detailed comments on all of the language features and technics you'll use. - I think it will be nice to have an additional documentation which will describe the basic setup for the development you use. All the toold you need to develop, compile, test and debug. - Also, some nice links for the basic tutorials on Rust types, concepts, etc. - I think, we should have detailed unit tests. It will help to understand the code better and we will have less bugs (hopefully). So I'm writing a document for the wiki to cover this *and* rust supports tests as part of the system - that will be all be included. Is that acceptable? And the final and big point I wanted to mention: - We should be prepared for a slow review process because we have quite limited recources in the team and a lot of work should be done (WebUI still has to be refactored to React, and it is only a small piece of the workload). Also, I think, it makes sense to have the smallest Rust PRs that can be put together as an independent unit. That's totally reasonable. I'll try to keep it to small parts and will build up as we go. See below for ideas on how I'll work toward this. But everything is just my opinion and I don't know what others think and if everyone is prepared to join it. I am feeling excited though :) Thanks, Simon P.S. check the contribution guide please. Espesially a part about 'rebase-force-push'. I think it is nice not to force push during the review process (and rebase-squash only after you got an ACK). Yep, I saw this change. I'll keep it in mind :) On 15 May 2019, at 19:37, Ludwig wrote: Hi William, this is a good starting point to discuss and decide how to move forward with logging (although it looks like you are already some steps ahead). I have no strong ties to existing logging format and if we do it in rust or not I don't care, but I do not want to use existing functionality for the sake af a new unified approach. First we need to define what is the purpose and usage of our logs, what do we want to keep or extend. I see these main area: - statistics - auditing (authentications, modifications) . debugging (for my usage the most important aspect) Next: what are the real problems - performance: yes, it would be nice to have less logging impact on performance, but I haven't seen real complaints and doing it differently does not guarantee better performance, we have debug levels (like tracing) which are not usable because they slow down everything, I do not think this will be resolved by just replacing the library - information: we continue to add information to the logs, and there are still open requests So: If I look at your suggestions I do like some and have concerns with others, and I am not sure if the priorities are right. You list a couple of tickets related to logging, but forgot others, eg: https://pagure.io/389-ds-base/issue/47822 - improve logging of ACL decisions https://pagure.io/389-ds-base/issue/48680 - enable logging for 3rd party libs https://pagure.io/389-ds-base/issue/50002 - improve password policy logging https://pagure.io/389-ds-base/issue/50187 - improve replication logging These are all things that you you are correct, that I missed. So I want to focus on your point about what we want from logs, IE stats, auditing, debugging. I think this is a great summary of what we want, but to make it more detailed: * Stats about operations from start to finish * Auditing of client operations * Debugging of client operations * Debugging of internal server machinery and state * Reporting of errors outside of the normal operation system. Today we current have: access - auditing of operations at a highlevel audit - auditing of modifications and writes error - debugging of internal server machinery (we tend not to enable the levels of debugging operations ...) So I think there is room to improve. Now I think that performance so far is only a barrier in terms of preventing us from adding more detail, because it slows down operations. This is why it's pretty high on the list of "to fix". these requests for improvements of logging exist for quite some time, they were accepted as useful and good to have, but we didn't have the capacity to work on them. The work is tedious, going thru ACL or replication code and
[389-devel] Re: Advice on a memory issue
Hi William, |It looks to me that attr_syntax_create overwrite the allocated asi with one it allocates itself based on provided params. In short I think attr_syntax_creates allocates for you the syntaxinfo, you do not need to provide one. best regards thierry | On 5/16/19 8:17 AM, William Brown wrote: https://pagure.io/389-ds-base/pull-request/50379 This code is not yet ready to be merged. I'm currently having a problem with freeing the attrsyntaxinfo struct as part of the test. If the code is as is I get: = ==98363==ERROR: LeakSanitizer: detected memory leaks Direct leak of 160 byte(s) in 1 object(s) allocated from: #0 0x7fbc40e28538 in calloc (/usr/lib64/libasan.so.5+0xec538) #1 0x7fbc40a34be6 in slapi_ch_calloc /home/william/development/389ds/ds/ldap/servers/slapd/ch_malloc.c:175 #2 0x40499c in attr_syntax_add_from_name /home/william/development/389ds/ds/test/libslapd/schema/filter_validate.c:25 #3 0x404b22 in test_libslapd_schema_filter_validate_simple /home/william/development/389ds/ds/test/libslapd/schema/filter_validate.c:56 #4 0x7fbc40d340d8 (/usr/lib64/libcmocka.so.0+0x50d8) Objects leaked above: 0x60e00d60 (160 bytes) Direct leak of 160 byte(s) in 1 object(s) allocated from: #0 0x7fbc40e28538 in calloc (/usr/lib64/libasan.so.5+0xec538) #1 0x7fbc40a34be6 in slapi_ch_calloc /home/william/development/389ds/ds/ldap/servers/slapd/ch_malloc.c:175 #2 0x40499c in attr_syntax_add_from_name /home/william/development/389ds/ds/test/libslapd/schema/filter_validate.c:25 #3 0x404b0f in test_libslapd_schema_filter_validate_simple /home/william/development/389ds/ds/test/libslapd/schema/filter_validate.c:55 #4 0x7fbc40d340d8 (/usr/lib64/libcmocka.so.0+0x50d8) Objects leaked above: 0x60e00ba0 (160 bytes) SUMMARY: AddressSanitizer: 320 byte(s) leaked in 2 allocation(s). However, if I free *a and *b, with attr_syntax_free, or slapi_ch_free, I get a double free error. The size of 160 bytes correlates to the sizeof(struct attrsyntaxinfo) but looking in gdb during attr_syntax_delete, the attr_syntax_free is called on asi as provided. So I'm not 100% sure what's going wrong here, but I'm not thoroughly experienced in this part of the code, so feedback would be really helpful about this resource issue. Thanks! — Sincerely, William Brown Senior Software Engineer, 389 Directory Server SUSE Labs ___ 389-devel mailing list -- 389-devel@lists.fedoraproject.org To unsubscribe send an email to 389-devel-le...@lists.fedoraproject.org Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/389-devel@lists.fedoraproject.org ___ 389-devel mailing list -- 389-devel@lists.fedoraproject.org To unsubscribe send an email to 389-devel-le...@lists.fedoraproject.org Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/389-devel@lists.fedoraproject.org
[389-devel] Advice on a memory issue
https://pagure.io/389-ds-base/pull-request/50379 This code is not yet ready to be merged. I'm currently having a problem with freeing the attrsyntaxinfo struct as part of the test. If the code is as is I get: = ==98363==ERROR: LeakSanitizer: detected memory leaks Direct leak of 160 byte(s) in 1 object(s) allocated from: #0 0x7fbc40e28538 in calloc (/usr/lib64/libasan.so.5+0xec538) #1 0x7fbc40a34be6 in slapi_ch_calloc /home/william/development/389ds/ds/ldap/servers/slapd/ch_malloc.c:175 #2 0x40499c in attr_syntax_add_from_name /home/william/development/389ds/ds/test/libslapd/schema/filter_validate.c:25 #3 0x404b22 in test_libslapd_schema_filter_validate_simple /home/william/development/389ds/ds/test/libslapd/schema/filter_validate.c:56 #4 0x7fbc40d340d8 (/usr/lib64/libcmocka.so.0+0x50d8) Objects leaked above: 0x60e00d60 (160 bytes) Direct leak of 160 byte(s) in 1 object(s) allocated from: #0 0x7fbc40e28538 in calloc (/usr/lib64/libasan.so.5+0xec538) #1 0x7fbc40a34be6 in slapi_ch_calloc /home/william/development/389ds/ds/ldap/servers/slapd/ch_malloc.c:175 #2 0x40499c in attr_syntax_add_from_name /home/william/development/389ds/ds/test/libslapd/schema/filter_validate.c:25 #3 0x404b0f in test_libslapd_schema_filter_validate_simple /home/william/development/389ds/ds/test/libslapd/schema/filter_validate.c:55 #4 0x7fbc40d340d8 (/usr/lib64/libcmocka.so.0+0x50d8) Objects leaked above: 0x60e00ba0 (160 bytes) SUMMARY: AddressSanitizer: 320 byte(s) leaked in 2 allocation(s). However, if I free *a and *b, with attr_syntax_free, or slapi_ch_free, I get a double free error. The size of 160 bytes correlates to the sizeof(struct attrsyntaxinfo) but looking in gdb during attr_syntax_delete, the attr_syntax_free is called on asi as provided. So I'm not 100% sure what's going wrong here, but I'm not thoroughly experienced in this part of the code, so feedback would be really helpful about this resource issue. Thanks! — Sincerely, William Brown Senior Software Engineer, 389 Directory Server SUSE Labs ___ 389-devel mailing list -- 389-devel@lists.fedoraproject.org To unsubscribe send an email to 389-devel-le...@lists.fedoraproject.org Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/389-devel@lists.fedoraproject.org
