Gramps 5.0 Decisions

classic Classic list List threaded Threaded
55 messages Options
123
Reply | Threaded
Open this post in threaded view
|

Re: Gramps 5.0 Decisions

Josip
20. 08. 2015. u 0:35, Tim Lyons je napisao/la:
> Like 93% of Gramps users[1], I am not running on Linux, so I don't really
> have a choice of which DBMS I use, it is decided by the packagers.
>

Windows Gramps-4.2.0 users have all they need to try Gramps development
version and their backends.

>
> I was initially very sceptical about sqlite, especially because I seem to
> recall that someone on this mailing list said that they did not regard
> sqlite as a serious DBMS, but only suitable for toy use.

Toy compared to BSDDB :-)


--
Josip

------------------------------------------------------------------------------
_______________________________________________
Gramps-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gramps-devel
Reply | Threaded
Open this post in threaded view
|

Re: Gramps 5.0 Decisions

Tim Lyons
Administrator
In reply to this post by DS Blank
I found the video referenced here
http://gramps.1791082.n4.nabble.com/Display-update-after-dismissing-a-dialog-tp4672434p4672494.html
I.e. https://www.youtube.com/watch?v=ON0A1dsQOV0
very interesting. It has got lots of really good examples of their UI problems with gtk.

There seem to be a lot of analogies between Gramps/BSDDB/GTK and sqlite/QT.

Gramps sees itself as primarily Linux, and only contemplates Mac and Windows under sufferance (they are 'community supported', not part of the 'core') despite the fact that up to 93% of our users are not Linux. This is just the same as the video points out about gtk (they have no interest in Mac or Windows ports). They also have many more non-Linux users than Linux ones. But they seem to constantly build all three versions at once.

BSDDB seems to be complicated and not really to have very good user facing documentation (just like the video says about gtk in contrast to QT). I think they mention what a struggle it is to find the right place in the gtk websites, just like with BSDDB, and the multiple different versions. They found it really difficult to use gtk correctly, or even to understand what correct was, just as we do with BSDDB (and gtk!).

I think we have many of the same problems with our using of gtk as the people in the video (the problem of how to get into an editing state - having to double click to open a new editing window is exactly the same as with gramps).

It was clear that for them switching between gtk and qt was a mammoth amount of effort, and they had lots of support and many really good developers. So unfortunately I think it would be out of the question for us, nice though it might be.

As I say, just a really interesting watch.

Tim.
Reply | Threaded
Open this post in threaded view
|

Re: Gramps 5.0 Decisions

Paul Franklin-5
In reply to this post by DS Blank
> One of default ones: Statistics. That can be easily give more useful
> stats... do you really need a full person table scan every time you open
> the tree to see that there are 16 people with Unknown genders? And yet that
> is the default.

I would rather see the default changed from Statistics to FAQ
(so the initial user sees FAQ and Welcome).

------------------------------------------------------------------------------
_______________________________________________
Gramps-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gramps-devel
Reply | Threaded
Open this post in threaded view
|

Re: Gramps 5.0 Decisions

manzi.sam
CONTENTS DELETED
The author has deleted this message.
Reply | Threaded
Open this post in threaded view
|

Re: Gramps 5.0 Decisions

John Ralls-2
In reply to this post by Tim Lyons

> On Aug 19, 2015, at 11:35 PM, Tim Lyons <[hidden email]> wrote:
>
> [1] Current figures for 4.1.3 are:
>
> Mac (Intel + PPC)       9%
> Windows (32+64 bit)  84%
> Linux                          8%
>
> (OK, some people may be downloading from places other than Sourceforge, but
> the figures are probably roughly right).
>

Tim,

That’s not a valid assumption. Nearly all Linux users get Gramps from their distro’s package system (e.g. apt-get or yum), downloaded from their distro’s package repository (or the host distro’s repo in the case of parasitic distros like Arch and Mint). Those downloads are invisible to us. The ones we see from SF are the packagers and the few unix experts who want to have the latest-and-greatest and are capable of installing from the tarball or Jérôme’s .deb.

Regards,
John Ralls


------------------------------------------------------------------------------
_______________________________________________
Gramps-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gramps-devel
Reply | Threaded
Open this post in threaded view
|

Re: Gramps 5.0 Decisions

John Ralls-2
In reply to this post by Josip

> On Aug 20, 2015, at 12:08 AM, Josip <[hidden email]> wrote:
>
> 20. 08. 2015. u 0:35, Tim Lyons je napisao/la:
>> Like 93% of Gramps users[1], I am not running on Linux, so I don't really
>> have a choice of which DBMS I use, it is decided by the packagers.
>>
>
> Windows Gramps-4.2.0 users have all they need to try Gramps development
> version and their backends.

SqLite3 is also part of the Mac bundle (Gramps.app/Contents/Resources/lib/libsqlite3.0.dylib) because several dependencies require it.

>
>>
>> I was initially very sceptical about sqlite, especially because I seem to
>> recall that someone on this mailing list said that they did not regard
>> sqlite as a serious DBMS, but only suitable for toy use.
>
> Toy compared to BSDDB :-)

Neither is a toy, but Berkeley DB* (BDB) is (or was, before Oracle introduced a SQL API in 5.0) a very low-level library. MySQL uses BDB as one of their available backends; in fact, MySQL wasn’t ACID (https://en.wikipedia.org/wiki/ACID) until they implemented the BDB backend (I’m pretty sure that they have made their other backends ACID since then). All of the relational superstructure that makes SQL easy to use has to be implemented in code with BDB. What’s more, making a BDB implementation ACID requires special coding too. SQLite on the other hand is a full relational database, so much less custom code is required to get it to work.

That said, our database design where we bundle up the persistent objects as BLOBs is non-relational. Switching to SQLite3 while maintaining that design wastes most of SQLite3’s capabilities and requires that we continue to hand-code indexes, referential integrity, database transactions, and queries. If, as Tim seems to think, we’re doing any of that wrong switching to SQLite isn’t going to fix it without a substantial redesign.

Regards,
John Ralls

*BSDDB is a misnomer. Berkeley DB did begin as part of the BSD in the 1980s but became a separate product before 1.0, first of Sleepycat Software and now Oracle after the latter acquired the former. It is dual-licensed under the GPL (AGPL since 6.0) or a commercial license, not the BSD license. See https://en.wikipedia.org/wiki/Berkeley_DB#Origin for more details.
------------------------------------------------------------------------------
_______________________________________________
Gramps-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gramps-devel
Reply | Threaded
Open this post in threaded view
|

Re: Gramps 5.0 Decisions

Tim Lyons
Administrator
John Ralls-2 wrote
That said, our database design where we bundle up the persistent objects as BLOBs is non-relational. Switching to SQLite3 while maintaining that design wastes most of SQLite3’s capabilities and requires that we continue to hand-code indexes, referential integrity, database transactions, and queries. If, as Tim seems to think, we’re doing any of that wrong switching to SQLite isn’t going to fix it without a substantial redesign.
Is it really true that we "hand-code [] database transactions"? [1]

I assumed that transactions were implemented by the Berkeley DB (BDB).

It says here:
https://gramps-project.org/wiki/index.php?title=Database_Formats#Gramps_2.2
that "Gramps 2.2 started using the transaction capability of the Berkeley database. This feature ensures that all related data is committed at once. So, if an error occurs while that data is being saved, the database remains intact. Either the entire set of changes makes it to the database, or none of the changes make it. " Is that no longer true, or is it miss-implemented?

It certainly seems to be the case (from various error reports) that if Gramps crashes (or is force quit) there is quite a good chance that the database is corrupted, and on restart the database is not automatically recovered to either before or after the complete set of changes. However I assumed this was because the restart was failing in some way. I never thought it might be because we were not using the BDB transaction system.

Regards,
Tim.

[1] The fact that we may write somewhere in the Gramps code "begin transaction" and 'end transaction" I do not regard as hand-coding transactions. That is just the syntax for specifying that a transaction is starting, and similar commands are apparently available in sqlite.
Reply | Threaded
Open this post in threaded view
|

Re: Gramps 5.0 Decisions

John Ralls-2

> On Aug 23, 2015, at 11:39 PM, Tim Lyons <[hidden email]> wrote:
>
> John Ralls-2 wrote
>> That said, our database design where we bundle up the persistent objects
>> as BLOBs is non-relational. Switching to SQLite3 while maintaining that
>> design wastes most of SQLite3’s capabilities and requires that we continue
>> to hand-code indexes, referential integrity, database transactions, and
>> queries. If, as Tim seems to think, we’re doing any of that wrong
>> switching to SQLite isn’t going to fix it without a substantial redesign.
>
> Is it really true that we "hand-code [] database transactions"? [1]
>
> I assumed that transactions were implemented by the Berkeley DB (BDB).
>
> It says here:
> https://gramps-project.org/wiki/index.php?title=Database_Formats#Gramps_2.2
> that "Gramps 2.2 started using the transaction capability of the Berkeley
> database. This feature ensures that all related data is committed at once.
> So, if an error occurs while that data is being saved, the database remains
> intact. Either the entire set of changes makes it to the database, or none
> of the changes make it. " Is that no longer true, or is it miss-implemented?
>
> It certainly seems to be the case (from various error reports) that if
> Gramps crashes (or is force quit) there is quite a good chance that the
> database is corrupted, and on restart the database is not automatically
> recovered to either before or after the complete set of changes. However I
> assumed this was because the restart was failing in some way. I never
> thought it might be because we were not using the BDB transaction system.
>
> Regards,
> Tim.
>
> [1] The fact that we may write somewhere in the Gramps code "begin
> transaction" and 'end transaction" I do not regard as hand-coding
> transactions. That is just the syntax for specifying that a transaction is
> starting, and similar commands are apparently available in sqlite.

Tim,

Transactions are more complicated than that, and since BDB is a low-level library a lot of transaction management must be done explicitly. There’s a lot to take into account [1]. SQL databases normally will transaction-protect single queries but Gramps’s design makes it difficult to wrap multiple-table operations into single queries. Hence my comment that we must continue to do that manually. Except that on examining the code for the BDB backend I find that we’re not, we’re only protecting individual table operations. Failing to ensure that multi-table operations are transactionally atomic leads to referential integrity problems.

Note that unhandled or improperly handled exceptions can result in transactions being left open, which will corrupt the database. That’s likely the source of many of our database corruption bugs.

Regards,
John Ralls

[1] Oracle doesn’t support Python, so we have to use the C++ documentation. The tutorial chapter on transactions is https://docs.oracle.com/cd/E17076_04/html/gsg_txn/CXX/BerkeleyDB-Core-Cxx-Txn.pdf and is 100 pages long. Not all of it is applicable since we don’t support multiple access (I hope Python isn’t creating threads behind our backs), but there’s still a lot that does.
------------------------------------------------------------------------------
_______________________________________________
Gramps-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gramps-devel
Reply | Threaded
Open this post in threaded view
|

Re: Gramps 5.0 Decisions

Gerald Britton-2

Actually I you can use any DBMS, including Oracle, via pyodbc, if the db has an odbc driver


On Mon, Aug 24, 2015, 6:07 AM John Ralls <[hidden email]> wrote:

> On Aug 23, 2015, at 11:39 PM, Tim Lyons <[hidden email]> wrote:
>
> John Ralls-2 wrote
>> That said, our database design where we bundle up the persistent objects
>> as BLOBs is non-relational. Switching to SQLite3 while maintaining that
>> design wastes most of SQLite3’s capabilities and requires that we continue
>> to hand-code indexes, referential integrity, database transactions, and
>> queries. If, as Tim seems to think, we’re doing any of that wrong
>> switching to SQLite isn’t going to fix it without a substantial redesign.
>
> Is it really true that we "hand-code [] database transactions"? [1]
>
> I assumed that transactions were implemented by the Berkeley DB (BDB).
>
> It says here:
> https://gramps-project.org/wiki/index.php?title=Database_Formats#Gramps_2.2
> that "Gramps 2.2 started using the transaction capability of the Berkeley
> database. This feature ensures that all related data is committed at once.
> So, if an error occurs while that data is being saved, the database remains
> intact. Either the entire set of changes makes it to the database, or none
> of the changes make it. " Is that no longer true, or is it miss-implemented?
>
> It certainly seems to be the case (from various error reports) that if
> Gramps crashes (or is force quit) there is quite a good chance that the
> database is corrupted, and on restart the database is not automatically
> recovered to either before or after the complete set of changes. However I
> assumed this was because the restart was failing in some way. I never
> thought it might be because we were not using the BDB transaction system.
>
> Regards,
> Tim.
>
> [1] The fact that we may write somewhere in the Gramps code "begin
> transaction" and 'end transaction" I do not regard as hand-coding
> transactions. That is just the syntax for specifying that a transaction is
> starting, and similar commands are apparently available in sqlite.

Tim,

Transactions are more complicated than that, and since BDB is a low-level library a lot of transaction management must be done explicitly. There’s a lot to take into account [1]. SQL databases normally will transaction-protect single queries but Gramps’s design makes it difficult to wrap multiple-table operations into single queries. Hence my comment that we must continue to do that manually. Except that on examining the code for the BDB backend I find that we’re not, we’re only protecting individual table operations. Failing to ensure that multi-table operations are transactionally atomic leads to referential integrity problems.

Note that unhandled or improperly handled exceptions can result in transactions being left open, which will corrupt the database. That’s likely the source of many of our database corruption bugs.

Regards,
John Ralls

[1] Oracle doesn’t support Python, so we have to use the C++ documentation. The tutorial chapter on transactions is https://docs.oracle.com/cd/E17076_04/html/gsg_txn/CXX/BerkeleyDB-Core-Cxx-Txn.pdf and is 100 pages long. Not all of it is applicable since we don’t support multiple access (I hope Python isn’t creating threads behind our backs), but there’s still a lot that does.
------------------------------------------------------------------------------
_______________________________________________
Gramps-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gramps-devel

------------------------------------------------------------------------------

_______________________________________________
Gramps-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gramps-devel
Reply | Threaded
Open this post in threaded view
|

Re: Gramps 5.0 Decisions

Nick Hall
In reply to this post by John Ralls-2
On 24/08/15 11:05, John Ralls wrote:
> Transactions are more complicated than that, and since BDB is a low-level library a lot of transaction management must be done explicitly. There’s a lot to take into account [1]. SQL databases normally will transaction-protect single queries but Gramps’s design makes it difficult to wrap multiple-table operations into single queries. Hence my comment that we must continue to do that manually. Except that on examining the code for the BDB backend I find that we’re not, we’re only protecting individual table operations. Failing to ensure that multi-table operations are transactionally atomic leads to referential integrity problems.
>
> Note that unhandled or improperly handled exceptions can result in transactions being left open, which will corrupt the database. That’s likely the source of many of our database corruption bugs.

For transactions, our code uses the DbTxn class in with-blocks. Using a
context manager ensures that the transaction will either be committed or
aborted when the block is exited.

We also abort a transactions if an attempt is made to initiate a nested
transaction, or if a transaction is still active when a database is closed.

The object commit methods are misleading because they actually update
records rather than commit transactions.

Which multi-table operations are you referring to?


Nick.


------------------------------------------------------------------------------
_______________________________________________
Gramps-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gramps-devel
Reply | Threaded
Open this post in threaded view
|

Re: Gramps 5.0 Decisions

Tim Lyons
Administrator
In reply to this post by John Ralls-2

On 24 Aug 2015, at 11:05, John Ralls wrote:

>
>> On Aug 23, 2015, at 11:39 PM, Tim Lyons <[hidden email]> wrote:
>>
>> John Ralls-2 wrote
>>> That said, our database design where we bundle up the persistent  
>>> objects
>>> as BLOBs is non-relational. Switching to SQLite3 while maintaining  
>>> that
>>> design wastes most of SQLite3’s capabilities and requires that we  
>>> continue
>>> to hand-code indexes, referential integrity, database  
>>> transactions, and
>>> queries. If, as Tim seems to think, we’re doing any of that wrong
>>> switching to SQLite isn’t going to fix it without a substantial  
>>> redesign.
>>
>> Is it really true that we "hand-code [] database transactions"? [1]
>>
>> I assumed that transactions were implemented by the Berkeley DB  
>> (BDB).
>>
>> It says here:
>> https://gramps-project.org/wiki/index.php?
>> title=Database_Formats#Gramps_2.2
>> that "Gramps 2.2 started using the transaction capability of the  
>> Berkeley
>> database. This feature ensures that all related data is committed  
>> at once.
>> So, if an error occurs while that data is being saved, the database  
>> remains
>> intact. Either the entire set of changes makes it to the database,  
>> or none
>> of the changes make it. " Is that no longer true, or is it miss-
>> implemented?
>>
>> It certainly seems to be the case (from various error reports) that  
>> if
>> Gramps crashes (or is force quit) there is quite a good chance that  
>> the
>> database is corrupted, and on restart the database is not  
>> automatically
>> recovered to either before or after the complete set of changes.  
>> However I
>> assumed this was because the restart was failing in some way. I never
>> thought it might be because we were not using the BDB transaction  
>> system.
>>
>> Regards,
>> Tim.
>>
>> [1] The fact that we may write somewhere in the Gramps code "begin
>> transaction" and 'end transaction" I do not regard as hand-coding
>> transactions. That is just the syntax for specifying that a  
>> transaction is
>> starting, and similar commands are apparently available in sqlite.
>
> Tim,
>
> Transactions are more complicated than that, and since BDB is a low-
> level library a lot of transaction management must be done  
> explicitly. There’s a lot to take into account [1]. SQL databases  
> normally will transaction-protect single queries but Gramps’s design  
> makes it difficult to wrap multiple-table operations into single  
> queries. Hence my comment that we must continue to do that manually.  
> Except that on examining the code for the BDB backend I find that  
> we’re not, we’re only protecting individual table operations.  
> Failing to ensure that multi-table operations are transactionally  
> atomic leads to referential integrity problems.
>
> Note that unhandled or improperly handled exceptions can result in  
> transactions being left open, which will corrupt the database.  
> That’s likely the source of many of our database corruption bugs.
>
> Regards,
> John Ralls
>
> [1] Oracle doesn’t support Python, so we have to use the C++  
> documentation. The tutorial chapter on transactions is https://docs.oracle.com/cd/E17076_04/html/gsg_txn/CXX/BerkeleyDB-Core-Cxx-Txn.pdf 
>  and is 100 pages long. Not all of it is applicable since we don’t  
> support multiple access (I hope Python isn’t creating threads behind  
> our backs), but there’s still a lot that does.


John,

Thanks for the reference, I have had a look at some of it.

I think I was misunderstanding 'managing' transactions as opposed to  
'implementing' transactions. I understand that BDB requires the  
application writer to manage transactions (i.e. start them, commit  
them and to specify for each operation the transaction it is to belong  
to, write checkpoints etc.). However, the application writer does not  
have to 'implement' transactions (i.e. he does not have to deal with  
log files etc.).

OK, so I understand that BDB is a low level library that requires a  
lot of coding to get it to work properly, whereas with sqlite, if you  
are just using the automatic transaction protect on single queries, it  
is all done automatically. (AIUI sqlite still has begin and end  
transaction methods, and these need to be used for the multi-table  
updates (see below))

> Note that unhandled or improperly handled exceptions can result in  
> transactions being left open, which will corrupt the database.  
> That’s likely the source of many of our database corruption bugs.

The with block structure (if it is coded consistently and correctly -  
which is a big if) should (as Nick points out) ensure that even if  
there is an unhanded or improperly handled exception the transaction  
is committed.

[Surely always committing is incorrect, if the with block is left  
because of an exception, the transaction should be aborted, but this  
doesn't matter either way with respect to database corruption, because  
it will only cause possible lack of referential integrity, not actual  
corruption]

Page 1 of the tutorial says:
"Your databases will never see a partially completed transaction. This  
is true even if your
application fails while there are in-progress transactions. If the  
application or system fails,
then either all of the database changes appear when the application  
next runs, or none of
them appear."

The question is, what happens if Gramps crashes, or is aborted (e.g.  
because it appears to be hanging) so that the __exit__ is not done. In  
this case, the database may be left corrupted, but should be restored  
(by the DB_RECOVER) once Gramps is started.


My understanding from the tutorial is that master  
gramps.plugins.database.bsddb_support.write.DbBsddb.load line 739  
might well be wrong. My reading is that specifying DB_PRIVATE means  
that "all region files are not backed by the filesystem, instead they  
are backed by heap memory". Surely this means that the DB_RECOVER flag  
that is also specified has no effect, because the necessary persistent  
data to make the DB ACID is not there. Our code comment says the  
DB_PRIVATE is used because the DB is not shared between users, but I  
think this is a misunderstanding of the purpose of the flag.

There was some discussion about this, for example in #7648:
"DB_PRIVATE
Specify that the environment will only be accessed by a single process  
(although that process may be multithreaded). This flag has two  
effects on the Berkeley DB environment. First, all underlying data  
structures are allocated from per-process memory instead of from  
shared memory that is potentially accessible to more than a single  
process. Second, mutexes are only configured to work between threads.  
This flag should not be specified if more than a single process is  
accessing the environment because it is likely to cause database  
corruption and unpredictable behavior. For example, if both a server  
application and the Berkeley DB utility db_stat are expected to access  
the environment, the DB_PRIVATE flag should not be specified."
So it seems that the rationale for using the DB_PRIVATE flag was that  
it would prevent the database being opened by two applications (or two  
instances of Gramps) - desirable (but it doesn't actually do that, and  
we code a db lock ourselves), whereas actually it prevents the  
environment being preserved across a crash.


I thought I recalled that there had been a problem with log files  
growing too large (e.g. for large imports or merges) and it had been  
decided to use an 'in-memory environment'. However, reading the  
tutorial it seems to me that any crash is liable to leave the database  
corrupt, which it not what we want (why bother to use transactions at  
all if they are not ACID). That is surely not what we wanted. The  
solution perhaps should be to periodically do a checkpoint during long  
operations to limit the size of log files (I expect you need to commit  
the transaction so far before you do the checkpoint, and I don't know  
how you would do that given the structure of the code).


Referential Integrity.

Referential integrity between <obj>_ref and the object it points to  
does not matter much. First, Gramps is everywhere carefully coded to  
allow for <obj>_ref that doesn't point to a real object. Everywhere we  
find the check missing (less and less now) we put one in. Second, the  
database check and repair (and actually similar checks in import) are  
specifically designed to find these dangling references and correct  
them. Third, all the database updates are carefully done so that the  
referenced object is inserted first (by a protected single table  
operation), and then the reference is inserted. This is to some extent  
just a consequence of the design of the editors that they operate on  
just a single object, and you have to find an existing object if you  
want to insert an <obj_ref>.  Inserting objects before the things that  
refer to them even happens with the imports which are done in a batch  
transaction (a multi table operation).

Possibly some of the gramplets may need to be checked to see whether  
abort could result in an inconsistent database.

With regard to the updates to the secondary indices, these are done  
within the object commit, using the same transaction as the object  
commit, so these are genuine multi-table operations, but are handled  
correctly (I think). (Don't know about sqlite backend).



sqlite is 'twice as slow'

AIUI sqlite is writing logs, and BDB is not (because the logs are just  
held in memory). If the display operations that were timed are running  
as write transactions, then BDB will just read the data whereas sqlite  
will have to write a log (and presumably just delete the file once the  
transaction ended). Could that account for a timing difference?


Regards,
Tim.


------------------------------------------------------------------------------
_______________________________________________
Gramps-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gramps-devel
Reply | Threaded
Open this post in threaded view
|

Re: Gramps 5.0 Decisions

Nick Hall
On 24/08/15 23:16, Tim Lyons wrote:
> Referential Integrity.
>
> Referential integrity between <obj>_ref and the object it points to
> does not matter much.

We should enforce referential integrity.

> First, Gramps is everywhere carefully coded to
> allow for <obj>_ref that doesn't point to a real object.

Gramps is coded like this everywhere that proxies can be used.  The
proxy can hide an object without hiding references to it.


> Everywhere we
> find the check missing (less and less now) we put one in. Second, the
> database check and repair (and actually similar checks in import) are
> specifically designed to find these dangling references and correct
> them.

Hopefully there should be no need to run the database check and repair tool.


> Third, all the database updates are carefully done so that the
> referenced object is inserted first (by a protected single table
> operation), and then the reference is inserted. This is to some extent
> just a consequence of the design of the editors that they operate on
> just a single object, and you have to find an existing object if you
> want to insert an <obj_ref>.  Inserting objects before the things that
> refer to them even happens with the imports which are done in a batch
> transaction (a multi table operation).

Correct.  This is how the editors work, so it is not essential to add
the object and its references in a single transaction.

If you look at the editor code, you will see the that delete queries
remove an object and its references in a single transaction.


Nick.


------------------------------------------------------------------------------
_______________________________________________
Gramps-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gramps-devel
Reply | Threaded
Open this post in threaded view
|

Re: Gramps 5.0 Decisions

John Ralls-2

> On Aug 24, 2015, at 11:53 PM, Nick Hall <[hidden email]> wrote:
>
> On 24/08/15 23:16, Tim Lyons wrote:
>> Referential Integrity.
>>
>> Referential integrity between <obj>_ref and the object it points to
>> does not matter much.
>
> We should enforce referential integrity.
>
>> First, Gramps is everywhere carefully coded to
>> allow for <obj>_ref that doesn't point to a real object.
>
> Gramps is coded like this everywhere that proxies can be used.  The
> proxy can hide an object without hiding references to it.
>
>
>> Everywhere we
>> find the check missing (less and less now) we put one in. Second, the
>> database check and repair (and actually similar checks in import) are
>> specifically designed to find these dangling references and correct
>> them.
>
> Hopefully there should be no need to run the database check and repair tool.
>
>
>> Third, all the database updates are carefully done so that the
>> referenced object is inserted first (by a protected single table
>> operation), and then the reference is inserted. This is to some extent
>> just a consequence of the design of the editors that they operate on
>> just a single object, and you have to find an existing object if you
>> want to insert an <obj_ref>.  Inserting objects before the things that
>> refer to them even happens with the imports which are done in a batch
>> transaction (a multi table operation).
>
> Correct.  This is how the editors work, so it is not essential to add
> the object and its references in a single transaction.
>
> If you look at the editor code, you will see the that delete queries
> remove an object and its references in a single transaction.
>

Tim, Nick,

Calm down. I was trying to explain to Tim why database transactions in BDB are not simple and need to be explicitly managed,  why pickling defeats many of the benefits of using a SQL database instead, and to speculate on reasons we see bug reports where run recovery isn’t succeeding. The last wasn’t intended as an attack, it was to suggest places where one could look for bugs. I think Tim’s observation that the logs aren’t getting flushed to disk enough and perhaps we aren’t checkpointing enough might have some bearing on that. Recover only works if it has logs.

That said, there are a bunch of functions in write.py where the with-DbTxn protection doesn’t appear to apply and where the exception handling wraps a single function rather than a DbTxn’s lifetime. If we’re not actually using that code then let’s remove it. Better yet, let’s remove it regardless and see if anything breaks: Whatever does then that can be rewritten to use with-DbTxn and we’ll have plugged a potential hole.

Another problem: BSDDBTxn.__exit__ says:
        if exc_type is not None:
            return False

IIUC that means that if an exception occurs in the with-DbTxn block then the txn is leaked rather than committed. Shouldn’t it abort the txn before returning False?


Regards,
John Ralls


------------------------------------------------------------------------------
_______________________________________________
Gramps-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gramps-devel
Reply | Threaded
Open this post in threaded view
|

Re: Gramps 5.0 Decisions

Nick Hall
On 25/08/15 00:28, John Ralls wrote:

> Calm down. I was trying to explain to Tim why database transactions in BDB are not simple and need to be explicitly managed,  why pickling defeats many of the benefits of using a SQL database instead, and to speculate on reasons we see bug reports where run recovery isn’t succeeding. The last wasn’t intended as an attack, it was to suggest places where one could look for bugs. I think Tim’s observation that the logs aren’t getting flushed to disk enough and perhaps we aren’t checkpointing enough might have some bearing on that. Recover only works if it has logs.
>
> That said, there are a bunch of functions in write.py where the with-DbTxn protection doesn’t appear to apply and where the exception handling wraps a single function rather than a DbTxn’s lifetime. If we’re not actually using that code then let’s remove it. Better yet, let’s remove it regardless and see if anything breaks: Whatever does then that can be rewritten to use with-DbTxn and we’ll have plugged a potential hole.
>
> Another problem: BSDDBTxn.__exit__ says:
>          if exc_type is not None:
>              return False
>
> IIUC that means that if an exception occurs in the with-DbTxn block then the txn is leaked rather than committed. Shouldn’t it abort the txn before returning False?
>
>

John,

I have been keeping an open mind and just been listening to the
discussion so far.  There have been some good points raised:

1. BSDDB is low level, so needs more care in its configuration and use.
2. Gramps uses the database as an object store not as a relational database.
3. We need to check that we are using transactions correctly.
Referential integrity is important.

What caught my attention was your comment about a bug in our existing
database code.  I think that you have identified a bug. The
BSDDBTxn.__exit__ method should abort the transaction when an exception
occurs.

My understanding is that a BSDDBTxn is used for things like metadata
where we do not need an undo history - otherwise a DbTxn is used.
Without going through the code in detail, it appears that all write
operations use transactions.

What functions in write.py are you concerned about in particular?  I
think that it is important that we fix any bugs in the database layer
and add some extra unit tests where appropriate.

If you have this under control, I'll leave you to it.


Nick.


------------------------------------------------------------------------------
_______________________________________________
Gramps-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gramps-devel
Reply | Threaded
Open this post in threaded view
|

Re: Gramps 5.0 Decisions

Tim Lyons
Administrator
In reply to this post by John Ralls-2
John Ralls-2 wrote
Calm down. I was trying to explain to Tim why database transactions in BDB are not simple and need to be explicitly managed,  why pickling defeats many of the benefits of using a SQL database instead, and to speculate on reasons we see bug reports where run recovery isn’t succeeding. The last wasn’t intended as an attack, it was to suggest places where one could look for bugs.
Sorry, I didn't interpret what you said as an attack, and wasn't intending to sound defensive. I found your comments helpful.

John Ralls-2 wrote
 I think Tim’s observation that the logs aren’t getting flushed to disk enough and perhaps we aren’t checkpointing enough might have some bearing on that. Recover only works if it has logs.
My point was that I think there is a bug in that the logs are not getting written to disk at all. They need to be written pretty constantly to support recovery; at any moment, either the log (or one of the associated log files) or the database itself needs to be consistent (i.e. not in the process of being updated), and one or the other needs to be written at every transaction to ensure the database can be recovered to the latest transaction. I think there is a long standing bug that nothing is written to dsik because DB_PRIVATE  is being used, and I don't think it should be used. Removing DB_PRIVATE may have an impact on performance and on disk space for logs, so needs to be carefully tested, but I hope that it might have an impact on our long running problems with database corruption.


John Ralls-2 wrote
That said, there are a bunch of functions in write.py where the with-DbTxn protection doesn’t appear to apply and where the exception handling wraps a single function rather than a DbTxn’s lifetime. If we’re not actually using that code then let’s remove it. Better yet, let’s remove it regardless and see if anything breaks: Whatever does then that can be rewritten to use with-DbTxn and we’ll have plugged a potential hole.
Can you point to a specific instance please - not sure what you mean.

Regards,
Tim.
Reply | Threaded
Open this post in threaded view
|

Re: Gramps 5.0 Decisions

enno
On 25-08-15 18:36, Tim Lyons wrote:

> John Ralls-2 wrote
>>   I think Tim’s observation that the logs aren’t getting flushed to disk
>> enough and perhaps we aren’t checkpointing enough might have some bearing
>> on that. Recover only works if it has logs.
> My point was that I think there is a bug in that the logs are not getting
> written to disk at all. They need to be written pretty constantly to support
> recovery; at any moment, either the log (or one of the associated log files)
> or the database itself needs to be consistent (i.e. not in the process of
> being updated), and one or the other needs to be written at every
> transaction to ensure the database can be recovered to the latest
> transaction. I think there is a long standing bug that nothing is written to
> dsik because DB_PRIVATE  is being used, and I don't think it should be used.
> Removing DB_PRIVATE may have an impact on performance and on disk space for
> logs, so needs to be carefully tested, but I hope that it might have an
> impact on our long running problems with database corruption.
AFAIK, logs files are definitely written, because I find lots in
database folders uploaded by reporters, and on my own PC. It's in fact
those logs that mess up the copy function, because they seem to contain
absolute paths to the files they were made for.

Since every database is opened with the DB_RECOVER flag, available logs
are automatically applied, leading to the messing up of the original
database after a copy, but also to automatic recovery when I try simple
corruptions like opening the same database twice with the same version
of Gramps, ignoring the lock warning. When I do that, and delete a
person in one copy, then exit both, and open the database again,
recovery always works.

Problems seem to occur when the 'environment' is messed up, whatever
that is, when a copy is made (see above), and when an upgrade fails, and
some tables are 4.1, while the others are still 3.4. And I bet that the
latter is related to the fact that the upgrade is not a single
transaction by itself, if such is at all possible.

regards,

Enno


------------------------------------------------------------------------------
_______________________________________________
Gramps-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gramps-devel
Reply | Threaded
Open this post in threaded view
|

Re: Gramps 5.0 Decisions

John Ralls-2
In reply to this post by Tim Lyons

On Aug 25, 2015, at 5:36 PM, Tim Lyons <[hidden email]> wrote:

> John Ralls-2 wrote
>> That said, there are a bunch of functions in write.py where the with-DbTxn
>> protection doesn’t appear to apply and where the exception handling wraps
>> a single function rather than a DbTxn’s lifetime. If we’re not actually
>> using that code then let’s remove it. Better yet, let’s remove it
>> regardless and see if anything breaks: Whatever does then that can be
>> rewritten to use with-DbTxn and we’ll have plugged a potential hole.
>
> Can you point to a specific instance please - not sure what you mean.


On Aug 25, 2015, at 2:42 PM, Nick Hall <[hidden email]> wrote:
> What functions in write.py are you concerned about in particular?  I think that it is important that we fix any bugs in the database layer and add some extra unit tests where appropriate.
>

For this one I’ll answer both of you together:

The cursor functions appear to me to not be wrapped in `with BSDDBTxn` blocks; what’s more, they’re passed self.txn as a transaction but it seems never to be initialized or committed. Finally, they have no exception handling except in their creating, and that’s only the @catch_db_error decorator which writes the log and re-raises.

load_tbl_txn() creates a transaction and commits it but has no exception handling. It’s also creating and committing a new txn for each put; it would be faster to create a single txn and run the loop with just the one.

commit_base() calls data_map.put and update_reference_map passing self.txn as the DbTxn; as pointend out above that doesn’t seem to be initialized or committed anywhere, and even if it were there’s no exception handling.

ISTM DbBsddb.txn should get removed. One doesn’t want to connect and disconnect from the database every time one wants to save something as that would be very expensive, but having a DbTxn member variable implies that one is using RAII [1], which would require destroying the DbBsddb object every time one wanted to commit.

Regards,
John Ralls

[1] https://en.wikipedia.org/wiki/Resource_Acquisition_Is_Initialization


------------------------------------------------------------------------------
_______________________________________________
Gramps-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gramps-devel
Reply | Threaded
Open this post in threaded view
|

Re: Gramps 5.0 Decisions

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

> On Aug 25, 2015, at 2:42 PM, Nick Hall <[hidden email]> wrote:
>
> On 25/08/15 00:28, John Ralls wrote:
>> Calm down. I was trying to explain to Tim why database transactions in BDB are not simple and need to be explicitly managed,  why pickling defeats many of the benefits of using a SQL database instead, and to speculate on reasons we see bug reports where run recovery isn’t succeeding. The last wasn’t intended as an attack, it was to suggest places where one could look for bugs. I think Tim’s observation that the logs aren’t getting flushed to disk enough and perhaps we aren’t checkpointing enough might have some bearing on that. Recover only works if it has logs.
>>
>> That said, there are a bunch of functions in write.py where the with-DbTxn protection doesn’t appear to apply and where the exception handling wraps a single function rather than a DbTxn’s lifetime. If we’re not actually using that code then let’s remove it. Better yet, let’s remove it regardless and see if anything breaks: Whatever does then that can be rewritten to use with-DbTxn and we’ll have plugged a potential hole.
>>
>> Another problem: BSDDBTxn.__exit__ says:
>>         if exc_type is not None:
>>             return False
>>
>> IIUC that means that if an exception occurs in the with-DbTxn block then the txn is leaked rather than committed. Shouldn’t it abort the txn before returning False?
>>
>>
>
> John,
>
> I have been keeping an open mind and just been listening to the discussion so far.  There have been some good points raised:
>
> 1. BSDDB is low level, so needs more care in its configuration and use.
> 2. Gramps uses the database as an object store not as a relational database.
> 3. We need to check that we are using transactions correctly. Referential integrity is important.
>
> What caught my attention was your comment about a bug in our existing database code.  I think that you have identified a bug. The BSDDBTxn.__exit__ method should abort the transaction when an exception occurs.

Yup. It certainly shouldn’t return False without doing anything as it does now, that just makes something outside of the transaction’s control handle it and leaks the transaction. Database corruption ensues.
>
> My understanding is that a BSDDBTxn is used for things like metadata where we do not need an undo history - otherwise a DbTxn is used. Without going through the code in detail, it appears that all write operations use transactions.

BSDDBTxn seems to be the dominant — and except for not aborting when there’s an exception — the most correct code in write.py for handling database puts.
>
> What functions in write.py are you concerned about in particular?  I think that it is important that we fix any bugs in the database layer and add some extra unit tests where appropriate.

Answered separately.

>
> If you have this under control, I'll leave you to it.

Sorry, I’m too busy with GnuCash to be able to take this on beyond offering advice.

Aborting the transaction in BSDDBTxn.__exit__() if there’s an unhandled exception (and I didn’t find any cases where exceptions are handled), losing Dbbsddb.txn and wrapping *all* database writes in `with BSDDBTxn` blocks will get you most of the way there. Cursor updates need some thought: Should each update be done in its own transaction or should the whole cursor operation be done in a single transaction and aborted if anything fails?

Consider checkpointing more frequently to ensure that data is flushed to the database frequently; that both reduces the likelihood of a database needing repair and makes any needed repairs simpler and quicker as well as keeping log file sizes under control.

Tim’s comment about buffering log files is also valid. No doubt it’s faster to let the OS sync the logs when it gets around to it, but it does reduce the ACIDity of the database.

Regards,
John Ralls




------------------------------------------------------------------------------
_______________________________________________
Gramps-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gramps-devel
Reply | Threaded
Open this post in threaded view
|

Re: Gramps 5.0 Decisions

Nick Hall
On 25/08/15 21:29, John Ralls wrote:
Aborting the transaction in BSDDBTxn.__exit__() if there’s an unhandled exception (and I didn’t find any cases where exceptions are handled), losing Dbbsddb.txn and wrapping *all* database writes in `with BSDDBTxn` blocks will get you most of the way there. 

I assume you are just talking about within write.py here.  Outside of write.py we use DbTxn.

We obviously need to fix the bug.


Cursor updates need some thought: Should each update be done in its own transaction or should the whole cursor operation be done in a single transaction and aborted if anything fails?

Gramps cursors are read-only.  This is why they do not use transactions.


Consider checkpointing more frequently to ensure that data is flushed to the database frequently; that both reduces the likelihood of a database needing repair and makes any needed repairs simpler and quicker as well as keeping log file sizes under control.

I agree.



Tim’s comment about buffering log files is also valid. No doubt it’s faster to let the OS sync the logs when it gets around to it, but it does reduce the ACIDity of the database.

I seem to remember this being discussed before, but I don't remember the reasoning.


Nick.


------------------------------------------------------------------------------

_______________________________________________
Gramps-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gramps-devel
Reply | Threaded
Open this post in threaded view
|

Re: Gramps 5.0 Decisions

Nick Hall
In reply to this post by John Ralls-2
On 25/08/15 21:04, John Ralls wrote:
> The cursor functions appear to me to not be wrapped in `with BSDDBTxn` blocks; what’s more, they’re passed self.txn as a transaction but it seems never to be initialized or committed. Finally, they have no exception handling except in their creating, and that’s only the @catch_db_error decorator which writes the log and re-raises.

Gramps cursors are read-only.


>
> load_tbl_txn() creates a transaction and commits it but has no exception handling. It’s also creating and committing a new txn for each put; it would be faster to create a single txn and run the loop with just the one.

This is new in gramps50 and I agree with you.  I haven't had a chance to
look at the new granps50 code yet.


>
> commit_base() calls data_map.put and update_reference_map passing self.txn as the DbTxn; as pointend out above that doesn’t seem to be initialized or committed anywhere, and even if it were there’s no exception handling.

The commit functions will be called inside 'with DbTxn' blocks.   I
don't see a problem here.


>
> ISTM DbBsddb.txn should get removed. One doesn’t want to connect and disconnect from the database every time one wants to save something as that would be very expensive, but having a DbTxn member variable implies that one is using RAII [1], which would require destroying the DbBsddb object every time one wanted to commit.

I haven't got time to investigate now, but I doubt that we are
connecting and disconnecting the database for every transaction.


Nick.


------------------------------------------------------------------------------
_______________________________________________
Gramps-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/gramps-devel
123