[ckan-dev] potential lost resource in case of 2 simultaneous requests ?

[Ian Ward]

Hi Alex,

Yes I can confirm this behaviour. It's made worse when creating a
resource involves uploading a large file because the window for losing
data becomes very large. I'm working on a new IUploader interface that
will address that part of the problem by uploading the file before
updating/creating a resource.

Using SELECT FOR UPDATE is a good approach for fixing the internal
race you describe. As Adrià said, an issue or pull request would be
great to see. We'll have to add a new validation error that catches
the conflict when raised from PostgreSQL.

Another related issue is e.g. a user opens a resource or dataset edit
page in their browser in the morning, then makes some changes and
submits them in the afternoon, while other users have made edits to
the same dataset or resource. I'd like to see some kind of optimistic
concurrency, like sending a hash of the values being replaced, to
prevent this kind of data loss.


On Tue, Jul 4, 2017 at 4:26 AM, Adrià Mercader <adria.mercader at okfn.org> wrote:
> Hi Alex,
>
> Thanks for your thorough investigations. I think that this is something
> worth discussing in the dev meeting. Would you mind creating an issue on the
> main repo with this information so we can discuss there?
>
> Thanks a lot.
>
> Adrià
>
> On 2 July 2017 at 01:27, Alex Gartner <alexandru.gartner+ckan at gmail.com>
> wrote:
>>
>> I've changed a bit package_show() and the get() function from Package
>> model to use "with_for_update()" from sqlalchemy. The idea was to use
>> "select for update" when retrieving the package so that another
>> request/transaction can't modify them in the meantime. Tested locally and it
>> seemed to work fine: prevented the resource from disappearing as in the
>> scenario from my initial email.
>>
>> package_show() - added support for a 'for_update' flag in context. This
>> flag was set when resource_create() / resource_update() started.:
>> for_update = context.get('for_update', False)
>> pkg = model.Package.get(name_or_id, for_update)
>>
>>
>> get() in Package model
>>
>> @classmethod
>> def get(cls, reference, for_update=False):
>>     '''Returns a package object referenced by its id or name.'''
>>
>>     query = meta.Session.query(cls).filter(cls.id==reference)
>>     if for_update:
>>         query = query.with_for_update()
>>
>>     pkg = query.first()
>>     if pkg == None:
>>         pkg = cls.by_name(reference)
>>     return pkg
>>
>>
>>
>>
>> On Sat, Jul 1, 2017 at 1:59 AM, Alex Gartner
>> <alexandru.gartner+ckan at gmail.com> wrote:
>>>
>>> Hello,
>>>
>>> I'm wondering if the fact that resource_create() ( and similarly
>>> resource_update() ) does a package_show() followed later by a
>>> package_update() can potentially lead to a lost resource in some special
>>> cases.
>>>
>>> Since postgres uses by default "read committed" transaction isolation I
>>> think the following could happen:
>>> 2 almost simultaneous requests (R1, R2) come to the API and are dealt
>>> with by different processes/threads. Both are modifying dataset D which
>>> already has one resource (resource1)
>>>
>>> TIMELINE
>>>
>>>  (R1) starts resource_create(resource2) on dataset D
>>>  (R2) starts resource_update(resource1) on dataset D
>>>  (R2) does package_show(D) => gets D with just resource1
>>>  (R2) changes resource1 in D
>>>  (R1) does package_show(D) => gets D with resource1
>>>  (R1) adds resource2 to D => D.resources = [resource1, resource2]
>>>  (R1) does package_update(D)
>>>  (R1) resource_create() finishes, everything is committed successfully to
>>> the db => D has 2 resources in the db
>>>  (R2) does package_update(D) - please note that here D only has one
>>> resource as read in step 3
>>>  (R2) resource_update() finishes, everything is committed successfully to
>>> the db => D has just resource1 (resource2 disappears)
>>>
>>> Question: is this something that seems possible ? I reproduced this
>>> locally on a slightly modified CKAN running paster but that could also mean
>>> that I have something misconfigured or changed. Before starting to think
>>> about strategies for avoiding this scenario (like a different transaction
>>> isolation) is there some mechanism in CKAN that would prevent this ? Did
>>> anyone stumble onto such an issue ?
>>>
>>> Thank you,
>>> Alex Gartner
>>>
>>
>>
>> _______________________________________________
>> ckan-dev mailing list
>> ckan-dev at lists.okfn.org
>> https://lists.okfn.org/mailman/listinfo/ckan-dev
>> Unsubscribe: https://lists.okfn.org/mailman/options/ckan-dev
>>
>
>
> _______________________________________________
> ckan-dev mailing list
> ckan-dev at lists.okfn.org
> https://lists.okfn.org/mailman/listinfo/ckan-dev
> Unsubscribe: https://lists.okfn.org/mailman/options/ckan-dev
>