Application level locking

classic Classic list List threaded Threaded
24 messages Options
12
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Application level locking

Nick Hall
Devs,

I have found an easy way to create a broken link:

1. In an empty database, create one person with one event.

2. Edit the person and leave the editor open.

3. Switch to the event view and delete the event.

4. Click "OK" in the person editor.

The editor will try to save a reference to the deleted event.  In 4.2,
this may be the cause of some of our NoneType errors.  In 5.0, we now
get a more descriptive HandleError.

The problem occurs because the delete action edits an object that is
already being edited.

At the moment, the only locking is provided by the ManagedWindow class
which prevents two editors from editing the same object. Locking is a
known issue for the census editor, which effectively edits an event and
all its participants.  Editing a census participant whilst the census
editor is open can result in data not being saved.

Multi-user access in the future will also require application level
locking.  Consider two users editing the same person in two different
sessions:

1. User A open the person editor.

2. User B edits the same person and saves their changes.

3. User A saves their changes.

The changes made by User B will be overwritten.

It look like we need to consider a better locking mechanism.

Any ideas or suggestions?

Regards,


Nick.



------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Gramps-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gramps-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Application level locking

prculley
I wonder if we might include as part of a commit, an option to validate references.  If before/after the base commit they were found to be invalid, the db could refuse the commit or undo and warn user...  If we ever go multi-user, this short block would have to be thread safe.  We'd probably want to fix dialogs so they don't close until the commit actually succeeded, so user could try to fix things.  Of course that would require yet more code to tell user what went wrong...

A second (perhaps better) possibility would be to validate, and quietly remove, any invalid references.  Again as part of commit code.  Could then complete the commit normally and leave db consistent.  This would be pretty non-invasive to everything else.  Probably want to avoid the validation if running batch mode...

More than this and it starts to sound like you need two phase commit strategy like used by the big multi-user dbs.

Paul Culley

On Mon, Feb 13, 2017 at 11:45 AM, Nick Hall <[hidden email]> wrote:
Devs,

I have found an easy way to create a broken link:

1. In an empty database, create one person with one event.

2. Edit the person and leave the editor open.

3. Switch to the event view and delete the event.

4. Click "OK" in the person editor.

The editor will try to save a reference to the deleted event.  In 4.2,
this may be the cause of some of our NoneType errors.  In 5.0, we now
get a more descriptive HandleError.

The problem occurs because the delete action edits an object that is
already being edited.

At the moment, the only locking is provided by the ManagedWindow class
which prevents two editors from editing the same object. Locking is a
known issue for the census editor, which effectively edits an event and
all its participants.  Editing a census participant whilst the census
editor is open can result in data not being saved.

Multi-user access in the future will also require application level
locking.  Consider two users editing the same person in two different
sessions:

1. User A open the person editor.

2. User B edits the same person and saves their changes.

3. User A saves their changes.

The changes made by User B will be overwritten.

It look like we need to consider a better locking mechanism.

Any ideas or suggestions?

Regards,


Nick.



------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Gramps-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gramps-devel


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Gramps-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gramps-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Application level locking

Paul Franklin-5
In reply to this post by Nick Hall
On 2/13/17, Nick Hall <[hidden email]> wrote:
> At the moment, the only locking is provided by the ManagedWindow class
> which prevents two editors from editing the same object.

Could you set the editors to be "modal" and prevent the
possibility of two editors even being created?

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Gramps-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gramps-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Application level locking

John Ralls-2

> On Feb 13, 2017, at 1:04 PM, Paul Franklin <[hidden email]> wrote:
>
> On 2/13/17, Nick Hall <[hidden email]> wrote:
>> At the moment, the only locking is provided by the ManagedWindow class
>> which prevents two editors from editing the same object.
>
> Could you set the editors to be "modal" and prevent the
> possibility of two editors even being created?

That does nothing for the multi-user situation and is fragile: It requires careful code review to ensure that you get all of the cases and never create new ones.

Locking correctly and enforcing referential integrity is less fragile and also handles multi-user.

Regards,
John Ralls
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Gramps-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gramps-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Application level locking

Nick Hall
In reply to this post by prculley
Paul,

This doesn't work for my census editor problem.  Consider the following:

1. Edit a census using the census (form) editor.
2. Edit a participant of the census using the standard person editor.
3. Enter census details for the person.
4. Save the census.
5. Save the person.

The census details for the person will be lost.  To avoid this we need a way to lock the census event and all its participants.  I was thinking of a simple lock table.

Nick.


On 13/02/17 20:25, Paul Culley wrote:
I wonder if we might include as part of a commit, an option to validate references.  If before/after the base commit they were found to be invalid, the db could refuse the commit or undo and warn user...  If we ever go multi-user, this short block would have to be thread safe.  We'd probably want to fix dialogs so they don't close until the commit actually succeeded, so user could try to fix things.  Of course that would require yet more code to tell user what went wrong...

A second (perhaps better) possibility would be to validate, and quietly remove, any invalid references.  Again as part of commit code.  Could then complete the commit normally and leave db consistent.  This would be pretty non-invasive to everything else.  Probably want to avoid the validation if running batch mode...

More than this and it starts to sound like you need two phase commit strategy like used by the big multi-user dbs.

Paul Culley

On Mon, Feb 13, 2017 at 11:45 AM, Nick Hall <[hidden email]> wrote:
Devs,

I have found an easy way to create a broken link:

1. In an empty database, create one person with one event.

2. Edit the person and leave the editor open.

3. Switch to the event view and delete the event.

4. Click "OK" in the person editor.

The editor will try to save a reference to the deleted event.  In 4.2,
this may be the cause of some of our NoneType errors.  In 5.0, we now
get a more descriptive HandleError.

The problem occurs because the delete action edits an object that is
already being edited.

At the moment, the only locking is provided by the ManagedWindow class
which prevents two editors from editing the same object. Locking is a
known issue for the census editor, which effectively edits an event and
all its participants.  Editing a census participant whilst the census
editor is open can result in data not being saved.

Multi-user access in the future will also require application level
locking.  Consider two users editing the same person in two different
sessions:

1. User A open the person editor.

2. User B edits the same person and saves their changes.

3. User A saves their changes.

The changes made by User B will be overwritten.

It look like we need to consider a better locking mechanism.

Any ideas or suggestions?



------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Gramps-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gramps-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Application level locking

John Ralls-2

> On Feb 13, 2017, at 3:16 PM, Nick Hall <[hidden email]> wrote:
>
> Paul,
>
> This doesn't work for my census editor problem.  Consider the following:
>
> 1. Edit a census using the census (form) editor.
> 2. Edit a participant of the census using the standard person editor.
> 3. Enter census details for the person.
> 4. Save the census.
> 5. Save the person.
>
> The census details for the person will be lost.  To avoid this we need a way to lock the census event and all its participants.  I was thinking of a simple lock table.

Nick,

That requires you to get closer to the iron than you want, because you have to ensure that exactly one module handles all access to the lock table and that that module is protected by a mutex. Even that will fail in multi-user access because each instance of Gramps will have its own mutex. Besides, locking alone isn't sufficient. You also want transaction support so that in the census editor case either both the census *and* the person records get saved or neither gets saved, and you want foreign key constraints to prevent dangling references between the two; that applies to the event-deletion case you led off with as well.

BDB and SQL provide facilities (locking, transactions, and foreign key constraints) that handle these problems safely and with much less work. Use them!

Note that different database engines do locking differently. For example in BDB you can lock individual rows; in SQLite3 the lock is on the whole database, while MySQL and PostgreSQL explicitly lock tables (they'll lock rows during queries, but you don't have enough control over that process to protect every case).

In the case of the census editor you also want a transaction that wraps both insert operations to make the whole thing atomic so that if writing the person.

For the earlier case of deleting the event out from under the person, it could be addressed with locking but foreign key constraints will work better and more generally.

Regards,
John Ralls


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Gramps-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gramps-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Application level locking

prculley
With regard to my earlier (validate on commit) proposals, my thoughts were more to preventing inconsistent db state than trying to maintain overlapping operations in some useful way.

I personally don't think that multi-user should ever be a direction that Gramps should go, but that is another discussion.

If you take multi-user off the table, then you are trying to protect a user from himself.

Don't the use of locks or similar mean that processes like Nicks Census form & Person details could not even be attempted?  Seems like the Census form would lock out the person editor from doing anything.

Some of our code already has callbacks from the db that attempts to inform the open dialog about changes; I was looking at the family editor and it has check_for_family_change method to deal with at least some changes relating to what it is editing.  Seems to me that making the Census form and similar cases work could be handled that way.  Again, would probably be rather hard to make sure you did not miss anything.  I'm pretty sure that in the single user world this can be workable.

Just some thoughts...

Paul C.

On Mon, Feb 13, 2017 at 5:48 PM, John Ralls <[hidden email]> wrote:

> On Feb 13, 2017, at 3:16 PM, Nick Hall <[hidden email]> wrote:
>
> Paul,
>
> This doesn't work for my census editor problem.  Consider the following:
>
> 1. Edit a census using the census (form) editor.
> 2. Edit a participant of the census using the standard person editor.
> 3. Enter census details for the person.
> 4. Save the census.
> 5. Save the person.
>
> The census details for the person will be lost.  To avoid this we need a way to lock the census event and all its participants.  I was thinking of a simple lock table.

Nick,

That requires you to get closer to the iron than you want, because you have to ensure that exactly one module handles all access to the lock table and that that module is protected by a mutex. Even that will fail in multi-user access because each instance of Gramps will have its own mutex. Besides, locking alone isn't sufficient. You also want transaction support so that in the census editor case either both the census *and* the person records get saved or neither gets saved, and you want foreign key constraints to prevent dangling references between the two; that applies to the event-deletion case you led off with as well.

BDB and SQL provide facilities (locking, transactions, and foreign key constraints) that handle these problems safely and with much less work. Use them!

Note that different database engines do locking differently. For example in BDB you can lock individual rows; in SQLite3 the lock is on the whole database, while MySQL and PostgreSQL explicitly lock tables (they'll lock rows during queries, but you don't have enough control over that process to protect every case).

In the case of the census editor you also want a transaction that wraps both insert operations to make the whole thing atomic so that if writing the person.

For the earlier case of deleting the event out from under the person, it could be addressed with locking but foreign key constraints will work better and more generally.

Regards,
John Ralls



------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Gramps-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gramps-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Application level locking

Nick Hall
In reply to this post by John Ralls-2
On 13/02/17 23:48, John Ralls wrote:
That requires you to get closer to the iron than you want, because you have to ensure that exactly one module handles all access to the lock table and that that module is protected by a mutex. Even that will fail in multi-user access because each instance of Gramps will have its own mutex. Besides, locking alone isn't sufficient. You also want transaction support so that in the census editor case either both the census *and* the person records get saved or neither gets saved, and you want foreign key constraints to prevent dangling references between the two; that applies to the event-deletion case you led off with as well.

BDB and SQL provide facilities (locking, transactions, and foreign key constraints) that handle these problems safely and with much less work. Use them!

Note that different database engines do locking differently. For example in BDB you can lock individual rows; in SQLite3 the lock is on the whole database, while MySQL and PostgreSQL explicitly lock tables (they'll lock rows during queries, but you don't have enough control over that process to protect every case).

In the case of the census editor you also want a transaction that wraps both insert operations to make the whole thing atomic so that if writing the person.

We already wrap all the save operations in a transaction.

The problem is more at the application rather than database level.  When we invoke an editor we make a copy of the object.  When the "OK" button is clicked we save this copy along with any changes made.

I don't think that extending transactions to cover the lifetime of an editor would do what we want.  A more usual solution would be to modify the editors to that they only update fields that are changed, but that is not easy.


For the earlier case of deleting the event out from under the person, it could be addressed with locking but foreign key constraints will work better and more generally.

At present, there are no foreign key constraints on our tables.  The reference fields are hidden within blobs.


Nick.



------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Gramps-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gramps-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Application level locking

John Ralls-2
In reply to this post by prculley

> On Feb 14, 2017, at 6:25 AM, Paul Culley <[hidden email]> wrote:
>
> With regard to my earlier (validate on commit) proposals, my thoughts were more to preventing inconsistent db state than trying to maintain overlapping operations in some useful way.
>
> I personally don't think that multi-user should ever be a direction that Gramps should go, but that is another discussion.
>
> If you take multi-user off the table, then you are trying to protect a user from himself.
>
> Don't the use of locks or similar mean that processes like Nicks Census form & Person details could not even be attempted?  Seems like the Census form would lock out the person editor from doing anything.
>
> Some of our code already has callbacks from the db that attempts to inform the open dialog about changes; I was looking at the family editor and it has check_for_family_change method to deal with at least some changes relating to what it is editing.  Seems to me that making the Census form and similar cases work could be handled that way.  Again, would probably be rather hard to make sure you did not miss anything.  I'm pretty sure that in the single user world this can be workable.
>
> Just some thoughts...

I'm ambivalent about multi-user. but we can legitimately relegate it to "distant future" and focus on maintaining database integrity in a single-process environment.

There are still good reasons to prefer the database engine's built-in locking and integrity features to rolling our own: They're written by developers who are experienced at it. There are other things that can go wrong with a database besides one window stomping on another, and using the database engine to enforce referential integrity and to enclose multi-step operations in a transaction so that all steps either succeed or fail together--without having to hand-code it into every operation--is one of the major advantages to using a database over a  plain file. Using the database's own locking means that a bit of code that forgets to check the lock still won't be able to stomp on an
operation that has the lock.

As for Nick's census plugin, he says that it "edits an event and all its participants"; that doesn't suggest to me that he's launching the person editor.  The census tool should acquire the lock on whatever objects it needs, begin a manual transaction, make its changes, commit the transaction, and relinquish the locks. To allow other windows to use the locked objects (remember that SQLite3 locks the whole database and only BDB can lock individual rows) while the user is editing (so she can look something up in another bit of the database perhaps), an edit window can take a read lock while the user works and change to a write lock (which will require all other edit windows to be closed because a write lock is exclusive) on "OK".

A more interesting problem is our UI design with cascading edit windows. Take the case of editing an event from inside the person editor: The person editor has taken a read lock when it's opened. The event editor does the same when it opens. No problem... until the user presses "OK" and the event editor tries to get a write lock. It can't because of the person editor's read lock. The solution that comes first to mind is for the "parent" edit window to be able to pass its read lock to child windows. The child window can then convert that lock into a write lock, commit their changes, and get a new read lock to pass back to the parent window. It's a bit clunky but it should work; on the other hand it's made a bit more complicated by the different locking models in the various database engines: BDB's row locking makes it completely unnecessary: child windows are editing a different row from the parent. SQLite3 locks the whole database so it's always required to pass a single lock around. MySQL and Postgres only need the lock passed when the child is operating on the same table (the only case I can think of is literally parent-child, where one edits a person's child from the children tab of the person editor).

It's true that all of this is to protect the user from himself. We can legitimately decide that this is all too much work for insufficient gain and not use locking. We can and should still use manual transactions to wrap multi-row operations and foreign-key constraints to ensure referential integrity. Together they prevent the scenario with which Nick opened this discussion.

Regards,
John Ralls


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Gramps-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gramps-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Application level locking

Nick Hall
In reply to this post by prculley
On 14/02/17 14:25, Paul Culley wrote:
> If you take multi-user off the table, then you are trying to protect a
> user from himself.

Yes.  The problem is that our editors take a copy of the object when the
editor is invoked.  Any change made outside the editor when it is open
will be overwritten when the editor changes are saved.

>
> Don't the use of locks or similar mean that processes like Nicks
> Census form & Person details could not even be attempted? Seems like
> the Census form would lock out the person editor from doing anything.

The window ID already provides a locking mechanism.  Editors set the ID
to the object handle.  If a user attempts to open two windows with the
same ID a WindowActiveError is emitted and and existing window is raised
to be made visible.

The census editor uses the event handle as the ID, so it locks the
event, but not the participants.  The delete operation doesn't use this
mechanism at all.


Nick.



------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Gramps-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gramps-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Application level locking

John Ralls-2
In reply to this post by Nick Hall

On Feb 14, 2017, at 7:21 AM, Nick Hall <[hidden email]> wrote:

On 13/02/17 23:48, John Ralls wrote:
That requires you to get closer to the iron than you want, because you have to ensure that exactly one module handles all access to the lock table and that that module is protected by a mutex. Even that will fail in multi-user access because each instance of Gramps will have its own mutex. Besides, locking alone isn't sufficient. You also want transaction support so that in the census editor case either both the census *and* the person records get saved or neither gets saved, and you want foreign key constraints to prevent dangling references between the two; that applies to the event-deletion case you led off with as well.

BDB and SQL provide facilities (locking, transactions, and foreign key constraints) that handle these problems safely and with much less work. Use them!

Note that different database engines do locking differently. For example in BDB you can lock individual rows; in SQLite3 the lock is on the whole database, while MySQL and PostgreSQL explicitly lock tables (they'll lock rows during queries, but you don't have enough control over that process to protect every case).

In the case of the census editor you also want a transaction that wraps both insert operations to make the whole thing atomic so that if writing the person.

We already wrap all the save operations in a transaction.

IIRC we wrap single row writes in transactions. This is OK in most cases because most of the editors work only on a single row at a time. Your census editor, OTOH, works on multiple rows in multiple tables and should wrap all of the writes in a single transaction. Does it?

The problem is more at the application rather than database level.  When we invoke an editor we make a copy of the object.  When the "OK" button is clicked we save this copy along with any changes made.

I don't think that extending transactions to cover the lifetime of an editor would do what we want.  A more usual solution would be to modify the editors to that they only update fields that are changed, but that is not easy.

It wouldn't. Opening a read lock when opening an editor and converting to a write lock on "OK" would, but it gets complicated as I explained in my previous post. It probably makes sense only if multi-user access is allowed.

As long as we're pickling objects it is impossible to update individual fields inside the pickle. As far as the database is concerned the pickle is the field. That's mostly OK with BDB since it's key-value anyway, but is a very bad design for SQL.

For the earlier case of deleting the event out from under the person, it could be addressed with locking but foreign key constraints will work better and more generally.

At present, there are no foreign key constraints on our tables.  The reference fields are hidden within blobs


I know, and it's a serious design flaw.

Regards,
John Ralls


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Gramps-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gramps-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Application level locking

Nick Hall
On 14/02/17 15:47, John Ralls wrote:
>> We already wrap all the save operations in a transaction.
>
> IIRC we wrap single row writes in transactions. This is OK in most
> cases because most of the editors work only on a single row at a time.
> Your census editor, OTOH, works on multiple rows in multiple tables
> and should wrap all of the writes in a single transaction. Does it?

Yes.  All objects are updated in a single transaction.

The problem is that when the person editor is invoked it takes a copy of
the person without locking it.  Then the census editor can modify the
person and commit it.  Finally the person editor will overwrite the data
entered in the census editor when "OK" is clicked.


Nick.



------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Gramps-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gramps-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Application level locking

John Ralls-2

> On Feb 14, 2017, at 9:24 AM, Nick Hall <[hidden email]> wrote:
>
> On 14/02/17 15:47, John Ralls wrote:
>>> We already wrap all the save operations in a transaction.
>>
>> IIRC we wrap single row writes in transactions. This is OK in most cases because most of the editors work only on a single row at a time. Your census editor, OTOH, works on multiple rows in multiple tables and should wrap all of the writes in a single transaction. Does it?
>
> Yes.  All objects are updated in a single transaction.
>
> The problem is that when the person editor is invoked it takes a copy of the person without locking it.  Then the census editor can modify the person and commit it.  Finally the person editor will overwrite the data entered in the census editor when "OK" is clicked.

Nick,

So in this case there's no foreign key violation. In that case the only straightforward solution is to lock. The alternative is to have a serial number on each object and before updating the code re-queries the object and checks that the serial number hasn't changed, raising an error if it has. That's even more cumbersome than locking.

Regards,
John Ralls
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Gramps-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gramps-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Application level locking

jerome
In reply to this post by Nick Hall
Hi,

I suppose that Family Editor is an other sample, where there
is pseudo 'real time' update, people handling and family object.

Despite be one primary Editor for different editions (person, family, childref),
maybe some corner cases remain.

In fact, a couple as family is just like a person ref association
(secondary record). In theory, the family (object) starts with the first
child or specific relation types. So, in a perfect world we should
be able to call the ChildEdition[1] before, after, during or within FamilyEdition
instead of a part of it. Same for some updates via a rebuild.

So, one big (flat) form has some limits too, and multiple editors
making the focus on one record are sometimes more adapted
for edition (vs display).

[1] https://github.com/gramps-project/gramps/pull/347#issuecomment-278995373


Jérôme

------------------------------------------------------------------------------
 Check out the vibrant tech community on one of
 the world's most
 engaging tech sites,
 SlashDot.org! http://sdm.link/slashdot
 _______________________________________________
 Gramps-devel mailing list
 [hidden email]
 https://lists.sourceforge.net/lists/listinfo/gramps-devel
 

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Gramps-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gramps-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Application level locking

Nick Hall
In reply to this post by John Ralls-2
On 14/02/17 22:21, John Ralls wrote:
We already wrap all the save operations in a transaction.
IIRC we wrap single row writes in transactions. This is OK in most cases because most of the editors work only on a single row at a time. Your census editor, OTOH, works on multiple rows in multiple tables and should wrap all of the writes in a single transaction. Does it?
Yes.  All objects are updated in a single transaction.

The problem is that when the person editor is invoked it takes a copy of the person without locking it.  Then the census editor can modify the person and commit it.  Finally the person editor will overwrite the data entered in the census editor when "OK" is clicked.
Nick,

So in this case there's no foreign key violation. In that case the only straightforward solution is to lock. The alternative is to have a serial number on each object and before updating the code re-queries the object and checks that the serial number hasn't changed, raising an error if it has. That's even more cumbersome than locking.

The last changed timestamp could be used for this.  If the object is changed since the editor was invoked we could warn the user.

Alternatively, we could enhance the application level locking provided by the window manager.  It works well for editors that only edit a single primary object, but doesn't provide functionality for multiple object editors.

I have also experimented with updating the current version of an object with the changes made in an editor.  This is actually quite easy using the json representation of our objects.

Nick.



------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Gramps-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gramps-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Application level locking

Nick Hall
In reply to this post by jerome
On 15/02/17 07:31, jerome wrote:

> I suppose that Family Editor is an other sample, where there
> is pseudo 'real time' update, people handling and family object.
>
> Despite be one primary Editor for different editions (person, family, childref),
> maybe some corner cases remain.
>
> In fact, a couple as family is just like a person ref association
> (secondary record). In theory, the family (object) starts with the first
> child or specific relation types. So, in a perfect world we should
> be able to call the ChildEdition[1] before, after, during or within FamilyEdition
> instead of a part of it. Same for some updates via a rebuild.
>

The person-family links are unusual because they are bi-directional.

What happens if a child is added to a family in the family editor when
the child is also being edited?

Nick.


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Gramps-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gramps-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Application level locking

John Ralls-2
In reply to this post by Nick Hall

On Feb 15, 2017, at 8:50 AM, Nick Hall <[hidden email]> wrote:

On 14/02/17 22:21, John Ralls wrote:
We already wrap all the save operations in a transaction.
IIRC we wrap single row writes in transactions. This is OK in most cases because most of the editors work only on a single row at a time. Your census editor, OTOH, works on multiple rows in multiple tables and should wrap all of the writes in a single transaction. Does it?
Yes.  All objects are updated in a single transaction.

The problem is that when the person editor is invoked it takes a copy of the person without locking it.  Then the census editor can modify the person and commit it.  Finally the person editor will overwrite the data entered in the census editor when "OK" is clicked.
Nick,

So in this case there's no foreign key violation. In that case the only straightforward solution is to lock. The alternative is to have a serial number on each object and before updating the code re-queries the object and checks that the serial number hasn't changed, raising an error if it has. That's even more cumbersome than locking.

The last changed timestamp could be used for this.  If the object is changed since the editor was invoked we could warn the user.

If by "warn the user" you mean "rollback the commit and tell the user that the object has been changed and ask her to pick which version to keep", yes.

Alternatively, we could enhance the application level locking provided by the window manager.  It works well for editors that only edit a single primary object, but doesn't provide functionality for multiple object editors.

Checking at the UI level is fragile, as I pointed out to Paul F a couple of days ago. You've already pointed out several cases where the UI concurrency controls are insufficient. 

I have also experimented with updating the current version of an object with the changes made in an editor.  This is actually quite easy using the json representation of our objects.

That's the JSON object inside of a SQL field architecture, right?
If so, you're fooling yourself. Yes, you can query the field, which copies *all* of its contents to memory, and change only parts of the memory image. But the update query will write back *the whole field image*, not only the bits that you changed. If another window has changed something else in that SQL field it gets overwritten.

Regards,
John Ralls

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Gramps-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gramps-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Application level locking

John Ralls-2

On Feb 15, 2017, at 9:19 AM, John Ralls <[hidden email]> wrote:


On Feb 15, 2017, at 8:50 AM, Nick Hall <[hidden email]> wrote:

On 14/02/17 22:21, John Ralls wrote:
We already wrap all the save operations in a transaction.
IIRC we wrap single row writes in transactions. This is OK in most cases because most of the editors work only on a single row at a time. Your census editor, OTOH, works on multiple rows in multiple tables and should wrap all of the writes in a single transaction. Does it?
Yes.  All objects are updated in a single transaction.

The problem is that when the person editor is invoked it takes a copy of the person without locking it.  Then the census editor can modify the person and commit it.  Finally the person editor will overwrite the data entered in the census editor when "OK" is clicked.
Nick,

So in this case there's no foreign key violation. In that case the only straightforward solution is to lock. The alternative is to have a serial number on each object and before updating the code re-queries the object and checks that the serial number hasn't changed, raising an error if it has. That's even more cumbersome than locking.

The last changed timestamp could be used for this.  If the object is changed since the editor was invoked we could warn the user.

If by "warn the user" you mean "rollback the commit and tell the user that the object has been changed and ask her to pick which version to keep", yes.

I guess the most user-friendly thing we could do would be to read the updated record into memory and present some sort of conflict resolution UI to the user so that he can meld the two changes. Implementation would be a lot of work.

Are we perhaps chasing a straw man here? Have we ever had a bug report complaining that Gramps "lost data" because some user tried to edit the same thing in two places?

Regards,
John Ralls


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Gramps-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gramps-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Application level locking

Nick Hall
In reply to this post by John Ralls-2
On 15/02/17 17:19, John Ralls wrote:

>>
>> I have also experimented with updating the current version of an
>> object with the changes made in an editor.  This is actually quite
>> easy using the json representation of our objects.
>>
> That's the JSON object inside of a SQL field architecture, right?
> If so, you're fooling yourself. Yes, you can query the field, which
> copies *all* of its contents to memory, and change only parts of the
> memory image. But the update query will write back *the whole field
> image*, not only the bits that you changed. If another window has
> changed something else in that SQL field it gets overwritten.
>
We don't store JSON in the database. It is only used for import/export
at the moment.

I created a JSON patch of the changes made in the editor.  When saving
the object, I read the object again, apply the patch, and then write the
object back to the database.  The serialize module contains to_json and
from_json functions to convert an object to and from JSON.

See:  http://jsonpatch.com/

Nick.


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Gramps-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gramps-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Application level locking

Nick Hall
In reply to this post by John Ralls-2
On 15/02/17 17:29, John Ralls wrote:
> I guess the most user-friendly thing we could do would be to read the
> updated record into memory and present some sort of conflict
> resolution UI to the user so that he can meld the two changes.
> Implementation would be a lot of work.
>
> Are we perhaps chasing a straw man here? Have we ever had a bug report
> complaining that Gramps "lost data" because some user tried to edit
> the same thing in two places?
>

We won't see "lost data" at the moment because one editor edits one
object, and the window manager prevents opening two editors to edit the
same object.  The exceptions are the census/form editors and the data
entry gramplet.  Running a tool whilst an editor is open may also cause
a problem, but I don't think users would tend to do this.

GEPS 34 lists multiple object editors as an idea for reducing the number
of editors:

https://gramps-project.org/wiki/index.php?title=GEPS_034:_Improve_usability#Reduce_the_number_of_editors

I plan to extend my form editor to create and edit conclusion objects
from the source data.  However, I really need a way of preventing a user
editing these objects with a standard editor and the form editor
simultaneously.

Nick.



------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Gramps-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gramps-devel
12
Loading...