specify / specify7

Specify 7
https://www.specifysoftware.org/products/specify-7/
GNU General Public License v2.0
66 stars 36 forks source link

Autoincrementing formatter not locking all required tree tables #4148

Open melton-jason opened 1 year ago

melton-jason commented 1 year ago

Hi again,

I have another problem, when I try to edit the taxon tree (not the other trees), I have an error:

"Table 'taxontreedef' was not locked with LOCK TABLES"

In the logs I found: field "_tableName" does not exist in <class 'specifyweb.specify.models.Taxon'>

I cannot reproduce this error in my test database, it seems that the taxon tree is not locked correctly ? I checked using specify6 and it doesn’t look locked by another user (don’t know how to get this info in the database)

https://discourse.specifysoftware.org/t/error-when-editing-taxon-tree-in-specify7/1359

Describe the bug

The problem is occurring as a result of how Specify 7 handles auto-incrementing fields in a UIFormatter while creating a new tree record which has a field formatted as auto-incrementing. More specifically, it is occurring because during the time that the new record is being created, Specify is locking the table (Taxon in this case), and all tables which have foreign keys which reference the main table.

However, to determine which tree to insert the Taxon record into, Specify does a SQL Inner Join on the discipline and taxontreedef tables while the Taxon (and related) tables are locked.

From MariaDB Lock Tables Documentation

While a connection holds an explicit lock on a table, it cannot access a non-locked table. If you try, the following error will be produced: ERROR 1100 (HY000): Table 'tab_name' was not locked with LOCK TABLES

To Reproduce

To reproduce the error, a UIFormatter on the Taxon table must be established with a numeric, autoincrementing/autonumbering field.

For example, the following is a formatter for the text1 field which has the pattern TAX-###, where # will be any digit which is supposed to be automatically set.

  <format system="false" name="TEST_TAXON_INCR" class="edu.ku.brc.specify.datamodel.Taxon" fieldname="text1">
  <autonumber>edu.ku.brc.af.core.db.AutoNumberGeneric</autonumber>
    <field type="constant" size="3" value="TAX"/>
    <field type="separator" size="1" value="-"/>
    <field type="numeric" size="3" inc="true"/>
  </format>

Once the format has been established, attempting to create any Taxon record will result in the OperationalError: Table 'taxontreedef' was not locked with LOCK TABLES.

*Editing Taxon records did not raise the error.

Desktop:

sanitized_Specify_7_Crash_Report.txt

Reported By

Institut de Recherche pour le Développement

specifysoftware commented 1 year ago

This issue has been mentioned on Specify Community Forum. There might be relevant details there:

https://discourse.specifysoftware.org/t/error-when-editing-taxon-tree-in-specify7/1359/5

melton-jason commented 1 year ago

Developer Notes

Following the Traceback provided in the crash report, the error is being raised as the direct result of the following line:

https://github.com/specify/specify7/blob/2e5ca79dc119e2d92222c90615b894e927bd22ec/specifyweb/specify/uiformatters.py#L145

It makes sense we are receiving a database error here: Django QuerySets are lazy.

Due to the nature of the error: Table 'taxontreedef' was not locked with LOCK TABLES, I investigated which tables were locked at the time the error is being thrown.

+----------+--------------------------+--------+-------------+
| Database | Table                    | In_use | Name_locked |
+----------+--------------------------+--------+-------------+
| specify  | taxonattachment          |      1 |           0 |
| specify  | taxon                    |      2 |           0 |
| specify  | determination            |      1 |           0 |
| specify  | taxoncitation            |      1 |           0 |
| specify  | collectingeventattribute |      1 |           0 |
| specify  | commonnametx             |      1 |           0 |
+----------+--------------------------+--------+-------------+

To get a better idea of the SQL being performed, the query can be extracted from the QuerySet. Which equates to the following (truncated for brevity)

SELECT ... FROM `taxon` INNER JOIN `taxontreedef` ON (`taxon`.`TaxonTreeDefID` = `taxontreedef`.`taxontreedefid`) INNER JOIN `discipline` ON (`taxontreedef`.`taxontreedefid` = `discipline`.`TaxonTreeDefID`) WHERE (`taxon`.`text1` REGEXP BINARY ^(TAX)(\\-)([0-9]{3})$ AND `discipline`.`usergroupscopeid` = 3) ORDER BY `taxon`.`text1` DESC;

Thus the problem becomes apparent: We are locking taxon and all tables which reference taxon as a Foreign Key, but trying to perform an INNER JOIN on Discipline and TaxonTreeDef.

Following the Stack Trace, we can see the autonumber QuerySet is being determined by the following:

https://github.com/specify/specify7/blob/2e5ca79dc119e2d92222c90615b894e927bd22ec/specifyweb/specify/uiformatters.py#L32-L45

In the case for Taxon, scope_info is None so it is using the default Collection QuerySet determined by the filter_by_collection(...) function.

Where the INNER JOINs is being inserted becomes apparent: https://github.com/specify/specify7/blob/2e5ca79dc119e2d92222c90615b894e927bd22ec/specifyweb/specify/filter_by_col.py#L30-L31

Furthermore, the locking of the tables is occuring in the following lines: https://github.com/specify/specify7/blob/2e5ca79dc119e2d92222c90615b894e927bd22ec/specifyweb/specify/autonumbering.py#L41-L43

In this case, obj._meta.db_table is taxon, so we are running the following SQL to perform the table locks: lock tables taxon write, which does exactly lock the tables as first mentioned.

The fix here would be to additionally lock the tables needed to determine the scoping for tree records.

:warning: This is yet to be confirmed, but the same problem likely occurs with the other trees in Specify 7:

maxpatiiuk commented 9 months ago

The above comments seem to be all about tree tables, but I am having this bug with CollectionObject:

When trying to create a blank Collection Object, Specify crashes with "Table 'institution' was not locked with LOCK TABLES")" error. Seems to happen during autonumbering according to stack trace

https://github.com/specify/specify7/assets/40512816/63dc9ffd-338a-4adc-bbbe-32dd3dd8f5f4

KU Fish Voucher Database

Not able to replicate in that database on the test panel - thus, maybe not a high priority issue, but opening this issue just in case Able to replicate after restarting the local docker database container

Specify 7 Crash Report - 2024-02-14T03_35_21.881Z.txt

melton-jason commented 9 months ago

You may be experiencing #4430, which was fixed in #4431. The cause of the issue was was a bug in running the second business rule migration (which applied the default uniqueness rules). The erroneous migration (before #4431) was likely already applied when #4431 was merged and because the migration was already stored in the django_migrations table, the updated migration was never run.

4431 was merged before v7.9.3 was released, so this issue should only occur for those use production in the time frame #4430 was introduced and #4431 was merged.

To resolve the issue locally, you can do the following in the database:

delete from django_migrations where app like "%business%";
drop table uniquenessrule_fields;
drop table uniquenessrule;

And then re-run migrations, either by restarting the backend container or manually via the following command (assuming your backend container is titled specify7-specify7-1):

docker exec -it specify7-specify7-1 bash -c "ve/bin/python3 manage.py migrate"
maxpatiiuk commented 9 months ago

thanks for the detailed instructions - I will go ahead and nuke the database entirely as there have been several bugs recently that are not reproducible on the test panel

emenslin commented 3 months ago

Can recreate in edge (7.9.6)