On 03/04/2014 03:27 AM, Noriko Hosoi wrote:
> Thank you, Ludwig.  A good catch!  I guess the change accidentally sneaked into when I was struggling to make the server run.  I'm going to delete the if-clause and run a set of tests one more time.
>
> Please note that I quickly scanned your original patch and my backported one to compare.  I think the place you pointed out is the only a significant difference...  Did you find any other fishy code in the patches I asked to review?
no, everything else looked good
>
> Thanks!!
> --noriko
>
> Ludwig Krispenz wrote:
>>
>> On 02/28/2014 06:54 PM, Noriko Hosoi wrote:
>>> Thank you, Ludwig!
>>>
>>> I cut out entry2str_internal_put_attrlist from my backported 1.2.11 branch (attached) and master, then compared the two.  They look identical to me...  If you could give me some more details, I'd greatly appreciate it.
>> yes, they are identical, but the change
>> if ( valueset_isempty(&a->a_deleted_values)) {
>> ...
>> }
>>
>> was introduced with ticket 569: https://fedorahosted.org/389/attachment/ticket/569/0001-New-Reduce-replication-meta-data-stored-in-entry-cf-tick.patch
>> it was not in the patch for ticket 346, so I am wondering why it is there, but I no longer see it in the patch, So I'm a bit puzzled now.
>>
>>> --noriko
>>>
>>>  Ludwig Krispenz wrote:
>>>> Hi Noriko,
>>>>
>>>> patch is ok, one question:
>>>> The change in  entry2str_internal_put_attrlist was introduced with fix for ticket #569, but this is not backported.
>>>>
>>>> Ludwig
>>>>
>>>> On 02/27/2014 06:29 PM, Noriko Hosoi wrote:
>>>>> Sorry, Ludwig.  I did attach a wrong one... :p  This is the core patch.
>>>>>
>>>>> I'm replacing this one with the new one with the formatting issue fixed.
>>>>>
>>>>>     0002-Ticket-346-version-4-Slow-ldapmodify-operation-time-.patch ​ <== old
>>>>>     some formatting issues here too, otherwise, ack 
>>>>>
>>>>>     https://fedorahosted.org/389/attachment/ticket/346/0001-Ticket-346-version-4-Slow-ldapmodify-operation-time-.patch
>>>>>
>>>>> Thanks!
>>>>> --noriko
>>>>>
>>>>> Ludwig Krispenz wrote:
>>>>>>
>>>>>> On 02/27/2014 05:47 PM, Noriko Hosoi wrote:
>>>>>>> Ludwig Krispenz wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> I cannot find the core 346 part  of the fix: 0002-Ticket-346-version-4-Slow-ldapmodify-operation-time-.patch​
>>>>>>>>
>>>>>>>> The other patches are ok.
>>>>>>> Hi Ludwig,
>>>>>>>
>>>>>>> Sorry about the confusion.  As I noted in this email, it was replaced with the new one with the formattiong issues fixed.
>>>>>>>>  0002-Ticket-346-version-4-Slow-ldapmodify-operation-time-.patch <=== old
>>>>>>>>  * some formatting issues here too, otherwise, ack
>>>>>>> https://fedorahosted.org/389/attachment/ticket/346/0001-Ticket-346-Slow-ldapmodify-operation-time-for-large-.2.patch <== new: could you please review this?
>>>>>> but this contains only the changes for slapi_attr_init_syntax(), but where are the changes for valueset.c and index.c .... ?
>>>>>>> formatting issues are fixed.
>>>>>>>
>>>>>>> Thanks!!
>>>>>>> --noriko
>>>>>>>>
>>>>>>>> Ludwig
>>>>>>>>
>>>>>>>> On 02/27/2014 02:34 AM, Noriko Hosoi wrote:
>>>>>>>>> Thank you for reviewing the changes, Rich.  I fixed the formatting issues and merged 2 patches into one.
>>>>>>>>>
>>>>>>>>> > Would like Ludwig to review these as well.
>>>>>>>>> Ludwig, could you please take a look?
>>>>>>>>>
>>>>>>>>> 389 Project wrote:
>>>>>>>>>> #346: Slow ldapmodify operation time for large quantities of multi-valued
>>>>>>>>>>
>>>>>>>>>> Comment (by rmeggins):
>>>>>>>>>>
>>>>>>>>>>  https://fedorahosted.org/389/attachment/ticket/346/0001-Ticket-346-Slow-
>>>>>>>>>>  ldapmodify-operation-time-for-large-.2.patch
>>>>>>>>>>  * the formatting is a little off - otherwise, ack
>>>>>>>>>  This patch could be merged into the one in ticket #47369.  I'm removing this one.  (I fixed the formatting issue is in #47369)
>>>>>>>>>>  0002-Ticket-346-version-4-Slow-ldapmodify-operation-time-.patch
>>>>>>>>>>  * some formatting issues here too, otherwise, ack
>>>>>>>>> https://fedorahosted.org/389/attachment/ticket/346/0001-Ticket-346-Slow-ldapmodify-operation-time-for-large-.2.patch
>>>>>>>>> formatting issues are fixed.
>>>>>>>>>>  0004-fix-coverity-11915-dead-code-introduced-with-fix-for.patch
>>>>>>>>>>  * ack
>>>>>>>>>>
>>>>>>>>>>  Would like Ludwig to review these as well.
>>>>>>>>>>
>>>>>>>>> 0003-Ticket-47369-version2-provide-default-syntax-plugin.patch​
>>>>>>>>>     git patch file (1.2.11) -- needed for backporting Ticket #346 "version 4 Slow ldapmodify operation time for large quantities of multi-valued attribute values" 
>>>>>>>>>
>>>>>>>>> 0005-Ticket-47455-valgrind-value-mem-leaks-uninit-mem-usa.patch​
>>>>>>>>>     git patch file (1.2.11) -- Backported the valueset.c part for Ticket #346 "version 4 Slow ldapmodify operation time for large quantities of multi-valued attribute values" 
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>