[PATCH 6 of 6 v2] bmstore: add handling of the active bookmark

[Pierre-Yves David]

On 12/02/2015 11:31 AM, Augie Fackler wrote:
>
>> On Dec 2, 2015, at 2:29 PM, Pierre-Yves David <pierre-yves.david at ens-lyon.org> wrote:
>>
>>
>>
>> On 12/02/2015 11:22 AM, Augie Fackler wrote:
>>>
>>>> On Dec 2, 2015, at 2:03 PM, Pierre-Yves David <pierre-yves.david at ens-lyon.org> wrote:
>>>>
>>>>
>>>>
>>>> On 12/02/2015 08:58 AM, Augie Fackler wrote:
>>>>> # HG changeset patch
>>>>> # User Augie Fackler <augie at google.com>
>>>>> # Date 1447294682 18000
>>>>> #      Wed Nov 11 21:18:02 2015 -0500
>>>>> # Node ID fa2189815d32e32c70c7c3d46c3ca60161ce5e95
>>>>> # Parent  6f3aa7041edf0ab6a4c891ebbf566b4c94523847
>>>>> bmstore: add handling of the active bookmark
>>>>
>>>> This active business have some transaction related issue (but you are not introducing them). However it surface as this patch adding more caller to bmstore.write(). We have been actively trying to kill 'bmstore.write' so this does not seems to be a direction we want to head to ☺
>>>
>>> It makes the problem more explicit, so I think it’s an improvement overall. I’d be happy to look into following this up with some more transaction-safe work here, but the goal of this cleanup was to contain all bookmarks things in a single container.
>>
>> Let me try a more explicit rephrasing of my previous point:
>>
>> 1) There was issue with transaction and they still exist after this patch. I'm fine with these issue to persist and work to be done later™
>
> Great!
>
>>
>> 2) This patch introduce a new user for 'bmstore.write()'. I see this as a blocker.
>
> In light of 1, you’re going to have to live with these write() calls until 1 can be cleaned up. Please don’t bloat the scope of this cleanup work. This patch makes the transactionality problem /more obvious/ and therefore more likely to get fixed.


The .write() function must die asap. We could get the logic related to 
writing the active bookmark in another function.

-- 
Pierre-Yves David