[Freeipa-devel] [PATCH] 1072 enable transaction support

[Martin Kosek]

On 11/20/2012 09:24 PM, Rob Crittenden wrote:
> Martin Kosek wrote:
>> On 11/16/2012 05:18 PM, Rob Crittenden wrote:
>>> Nalin Dahyabhai wrote:
>>>> On Thu, Nov 15, 2012 at 11:53:44PM -0500, Rob Crittenden wrote:
>>>>> In order for this to work you'll need to apply the last two patches
>>>>> (both 0001) to slapi-nis and spin it up yourself, otherwise you'll
>>>>> have serious deadlock issues. I know this is extra work but this
>>>>> patch is potentially disruptive so I figure the earlier it is out
>>>>> the better.
>>>>>
>>>>> Noriko/Rich/Nalin, can you guys review the slapi-nis pieces? I may
>>>>> have been too aggressive in my cleanup.
>>>>>
>>>>> Noriko/Rich, can you review the 389-ds plugin parts of my 1072 patch?
>>>>>
>>>>> Once we have an official slapi-nis build with these patches we'll
>>>>> need to set the minimum n-v-r in our spec file.
>>>>
>>>> Rob, the original patch was already applied.  I since reworked large
>>>> parts of how it was organized to make it easier for me to read, and
>>>> tagged the result as 0.43.  Have you tested the IPA changes in
>>>> combination with the 0.44 builds from either ipa-devel or Fedora 18's
>>>> updates-testing repository?
>>>>
>>>> Nalin
>>>>
>>>
>>> I tested the 0.44 build and things are looking good. I'm not deadlocking, so I
>>> guess that's the desired out come :-)
>>>
>>> I bulk loaded a few thousand users and groups and tested the compat and NIS
>>> plugins and the data appears correct.
>>>
>>> Updated patch with minimum n-v-r in spec attached.
>>>
>>> rob
>>>
>>
>> Good job, this closes a lot of tickets! I am now also able to run tests without
>> having to set wait_for_attr env config first!
>>
>> The patch generally seems to work OK, I tried it even with a replica without
>> transactions enabled and so far so good. I have found just few issues:
>>
>> 1) Patch needs a mild rebase (spec file conflict)
> 
> Done.
> 
>>
>> 2) One Unit test failure slipped:
>>
>> ======================================================================
>> FAIL: test_permission[22]: permission_find: Search for permissions by attr with
>> a limit of 1 (truncated)
> 
> I hadn't noticed this one because the memberof for the role wasn't being
> updated. I added a task that runs in the updates and that fixed it.
> 
>> 3) Question - what is the reason of keeping wait_for_attr sections in our code?
>> I may miss something, but I see no difference with api.env.wait_for_attr
>> enabled... AFAIU, there should not even be any difference as all attributes
>> should be filled in one transaction, so I would rather remove this flag from
>> both code and man page.
> 
> Was just hedging my bets. You're right though, it makes no sense when we have
> transactions. I've removed it.
> 
>> 4) nsslapd-pluginbetxn is not set for schema compatibility plugin after upgrade:
>>
>> # Schema Compatibility, plugins, config
>> dn: cn=Schema Compatibility,cn=plugins,cn=config
>> nsslapd-pluginId: schema-compat-plugin
>> cn: Schema Compatibility
>> objectClass: top
>> objectClass: nsSlapdPlugin
>> objectClass: extensibleObject
>> nsslapd-pluginDescription: Schema Compatibility Plugin
>> nsslapd-pluginEnabled: on
>> nsslapd-pluginPath: /usr/lib64/dirsrv/plugins/schemacompat-plugin.so
>> nsslapd-pluginVersion: 0.44 (betxn support available and enabled by default)
>> nsslapd-pluginVendor: redhat.com
>> nsslapd-pluginType: object
>> nsslapd-pluginInitfunc: schema_compat_plugin_init
>>
>> This is supposed to be enabled by default, judging by nsslapd-pluginVersion
>> description, but this may create an inconsistency between new installs and
>> upgraded IPA servers.
>>
>> The same issue applies to IPA server with NIS plugin enabled.
> 
> Yeah, I fixed the ldif but had not added an update for these.
> 
> I had to declare a new option for the updater, onlyifexist, which will force a
> value in an attribute only if the entry already exists. I need this to be sure
> that the only value for betxn is on.
> 
> rob
> 

Thanks. I think the patch works fine now.

ACK, pushed to master.

Martin