GitLab

This branch is for MariaDB >= 10.5 that does no longer have innodb_locks_unsafe_for_binlog.

To achieve a good performance in parallel processing, this branch uses following connections.

• cmf_activity_sql_connection: READ-COMMITTED isolation
• erp5_sql_connection: REPEATABLE-READ isolation
• erp5_sql_read_committed_connection: [NEW] READ-COMMITTED isolation
• erp5_sql_transactionless_connection: not using transaction

Basically, new erp5_sql_read_committed_connection is used for INSERT/DELETE/REPLACE operation. For certain cases for which we are sure that READ-COMMITTED isolation is fine, we can use it for SELECT operation as well, for example alarm table.

Also for certain cases for which we don't join with other tables, using the default erp5_sql_connection connection for everything is easier, for example syncml table.

Note that both READ-COMMITTED transaction and REPEATABLE-READ transaction start right after the Zope transaction starts, and READ-COMMITTED transaction's result cannot be fetched by REPEATABLE-READ transaction. Some unit tests failed by this reason and simply calling self.commit() will re-starts both MariaDB transactions.

mentioned in merge request slapos!1474

changed the description

There are also some potential problems with product/ERP5/bootstrap/erp5_core/SkinTemplateItem/portal_skins/erp5_core/IdTool_z*sql. Maybe it will just work, I wanted to mention this so that we don't forget this area.

Indeed, portal_ids query has its own explicit BEGIN..COMMIT query. We can specify the isolation level there if needed.

This is a good point, IdTool_zGenerateId is indeed incompatible with READ COMMITTED because it SELECTs after UPDATE and expects to get the value it just wrote.

Judging by how the code aliases the selected column to the same of a function, this looks like the code went from using this function to selecting the value from the table. But judging by the git history, such initial implementation was never committed, so there is no explanation as to why:

• it was changed (...although I guess this is some AUTOINCREMENT shenanigans)
• the code was not simplified to stop mentioning this function's name, as it is now a pointless alias (no backward compatibility should have been necessary, the product code which accesses the column could have accessed it as an index in the row instead of a named column)

There is an extension to INSERT and REPLACE which would be especially useful here (removing one query), but unfortunately it is only added in mariadb 10.5 and while we want to add support for 10.11 I do not think we are ready to break 10.3 . Also, I think this requires an especially tricky extension to ZMySQLDA so that it understands such query as requiring result rows have to be retrieved from the server (AFAIK it currently only does that when the query starts with SELECT, which is easy to detect, but with RETURNING the code will have to step over arbitrary values in the query and properly ignore them).

Which makes me think: have these changes been tested with 10.3 ?

added 1 commit

• 4340306a - fixup! ZMySQLDA: support isolation level per connector.

Compare with previous version

added 2 commits

• 4dc7f9d9 - fixup! erp5_mysql_innodb_catalog etc.: use erp5_sql_isolated_connection for non-SELECT methods.
• 0ee52ded - erp5_mysql_innodb_catalog etc.: rename sql_isolated_connection -> sql_read_committed_connection.

Compare with previous version

added 2 commits

• dab187ab - fixup! ZMySQLDA: support isolation level per connector.
• 72f5f2d7 - test: use explicit charset as utf8 is an alias for utf8mb3 form MariaDB 10.6.

Compare with previous version

added 1 commit

• 48dc8c7b - fixup! erp5_mysql_innodb_catalog etc.: rename sql_isolated_connection ->...

Compare with previous version

added 2 commits

• 2efb36f5 - fixup! ZMySQLDA: support isolation level per connector.
• d3484e8b - Revert "fixup! erp5_mysql_innodb_catalog etc.: rename sql_isolated_connection...

Compare with previous version

added 1 commit

• b034728d - fixup! ZMySQLDA: support isolation level per connector.

Compare with previous version

added 1 commit

• 4be54e1c - debug: z_get_table_schema should wait for table lock in case of site setup / upgrade ?

Compare with previous version

added 2 commits

• a29d125b - fixup! erp5_mysql_innodb_catalog etc.: rename sql_isolated_connection ->...
• 055d6546 - Revert "debug: z_get_table_schema should wait for table lock in case of site setup / upgrade ?"

Compare with previous version

added 5 commits

• 93b3bdc6 - test: use explicit charset as utf8 is an alias for utf8mb3 form MariaDB 10.6.
• 26676fb8 - ERP5Site: add erp5_sql_isolated_connection.
• a728dbbe - erp5_mysql_innodb_catalog etc.: use erp5_sql_isolated_connection for non-SELECT methods.
• 74d4341e - erp5_scalability_test: enable activity tracking log for deeper diagnostic.
• 453cdf79 - fixup! ERP5Site: add erp5_sql_isolated_connection.

Compare with previous version

added 1 commit

• bb7d064e - fixup! ERP5Site: add erp5_sql_isolated_connection.

Compare with previous version

added 2 commits

• 8be3b9da - fixup! erp5_mysql_innodb_catalog etc.: use erp5_sql_isolated_connection for non-SELECT methods.
• 3e4b0596 - fixup! ERP5Site: add erp5_sql_isolated_connection.

Compare with previous version

added 1 commit

• 4b477f6a - test: tic() at the end of each test, instead of long transaction for the whole test.

Compare with previous version

added 64 commits

• 4b477f6a...c42c1d38 - 53 commits from branch master
• cf2107a1 - test: use explicit charset as utf8 is an alias for utf8mb3 form MariaDB 10.6.
• f8633566 - ERP5Site: add erp5_sql_read_committed_connection.
• 62817550 - erp5_mysql_innodb_catalog etc.: use erp5_sql_read_committed_connection for non-SELECT methods.
• 10fe274e - erp5_syncml: use erp5_sql_connection for non-SELECT SQL as well.
• af8cf006 - erp5_scalability_test: enable activity tracking log for deeper diagnostic.
• d30f090f - testListBox: tic() at the end of each test, instead of long transaction for the whole test.
• edccff06 - testERP5Catalog: fix tests for erp5_sql_read_committed_connection.
• d95690fc - SQLCatalog: add read_committed argument in getConnectionId().
• 5165ac8a - erp5_archive: add READ-COMMITTED connection.
• cfa508aa - erp5_archive: remove unused arguments from manage_archive().
• e83a42ba - testAlarm: commit transaction after alarm.setNextAlarmDate().

Compare with previous version

added 1 commit

• fbcad2af - testInventoryAPI: commit Zope transaction at the beginning of beforeTearDown().

Compare with previous version

added 1 commit

• f198efab - testReturnedSalePackingList: commit Zope transaction before checking catalog results.

Compare with previous version

added 604 commits

• f198efab...27a6dce4 - 592 commits from branch master
• 02921c92 - ERP5Site: add erp5_sql_read_committed_connection.
• d7754519 - erp5_mysql_innodb_catalog etc.: use erp5_sql_read_committed_connection for non-SELECT methods.
• 70db7d80 - erp5_syncml: use erp5_sql_connection for non-SELECT SQL as well.
• 9806bd82 - erp5_scalability_test: enable activity tracking log for deeper diagnostic.
• 8ceb1ac0 - testListBox: tic() at the end of each test, instead of long transaction for the whole test.
• 78dc83ed - testERP5Catalog: fix tests for erp5_sql_read_committed_connection.
• 06ea6d6f - SQLCatalog: add read_committed argument in getConnectionId().
• ed6e4280 - erp5_archive: add READ-COMMITTED connection.
• b962c023 - erp5_archive: remove unused arguments from manage_archive().
• 68986971 - testAlarm: commit transaction after alarm.setNextAlarmDate().
• 450ac884 - testInventoryAPI: commit Zope transaction at the beginning of beforeTearDown().
• bf7d241a - testReturnedSalePackingList: commit Zope transaction before checking catalog results.

Compare with previous version

added 13 commits

• 85c5349a - ZMySQLDA: support isolation level per connector.
• db2e2368 - ERP5Site: add erp5_sql_read_committed_connection.
• f900ab62 - erp5_mysql_innodb_catalog etc.: use erp5_sql_read_committed_connection for non-SELECT methods.
• 111bb789 - erp5_syncml: use erp5_sql_connection for non-SELECT SQL as well.
• 72f87357 - erp5_scalability_test: enable activity tracking log for deeper diagnostic.
• 04f28f39 - testListBox: tic() at the end of each test, instead of long transaction for the whole test.
• 1e15a1a0 - testERP5Catalog: fix tests for erp5_sql_read_committed_connection.
• 12d5a4c1 - SQLCatalog: add read_committed argument in getConnectionId().
• 6bd9a313 - erp5_archive: add READ-COMMITTED connection.
• 5554ac05 - erp5_archive: remove unused arguments from manage_archive().
• f6cae4eb - testAlarm: commit transaction after alarm.setNextAlarmDate().
• e6238b5f - testInventoryAPI: commit Zope transaction at the beginning of beforeTearDown().
• bc14f4e7 - testReturnedSalePackingList: commit Zope transaction before checking catalog results.

Compare with previous version

added 2 commits

• d9eb2b26 - Revert "testAlarm: commit transaction after alarm.setNextAlarmDate()."
• 2d0c4df2 - erp5_core: use READ-COMMITTED transaction only in alarm table.

Compare with previous version

changed the description

added 2 commits

• 271cc7d9 - testERP5Catalog: add READ-COMMITTED connection in hot reindex test.
• f80e766f - testVanillaERP5Catalog: add READ-COMMITTED connection in hot reindex test.

Compare with previous version

added 2 commits

• 5c268d7e - fixup! erp5_mysql_innodb_catalog etc.: use erp5_sql_read_committed_connection...
• b3f19356 - erp5_core: use READ-COMMITTED connection only in deleted_catalog table.

Compare with previous version

added 1 commit

• 1ab40cdc - testInvalidationBug: generate transactionless connection string properly.

Compare with previous version

added 1 commit

• d26d9975 - testERP5Credential: commit transaction at the end of afterSetUp() to start new...

Compare with previous version

added 1 commit

• 6b55ac3e - testERP5CatalogSecurityUidOptimization: commit transaction after refreshWorklistCache().

Compare with previous version

changed the description

unmarked as a Work In Progress

changed title from WIP: Feat/mariadb 10.11 to Introducing READ-COMMITTED isolation connector

marked as a Work In Progress

added 1 commit

• 2740a235 - testTranslation: commit transaction before ERP5Site_updateTranslationTable().

Compare with previous version

added 1 commit

• a50d64c7 - testVanillaERP5Catalog, testIngestion: commit transaction before manage_catalogClear().

Compare with previous version

added 1 commit

• b89148e0 - testKM: commit transaction right after ERP5Site_addNewKnowledgePad().

Compare with previous version

added 5 commits

• 0f9c9e5c - testERP5Catalog: commit transaction before deletion to avoid deadlocks.
• d1bff15d - testSQLCachedWorklist: commit transaction before clear cache to avoid deadlocks.
• bbc86b63 - testWorklist: call tic() before worklist's setReference() that changes the ID...
• 9c5a085a - erp5_stock_cache: use erp5_sql_connection for non-SELECT SQL as well to avoid deadlocks.
• 7f5e9780 - fixup! erp5_mysql_innodb_catalog etc.: use erp5_sql_read_committed_connection...

Compare with previous version

I found difficulty to adapt erp5_stock_cache code to R-C + R-R combination.

SimulationTool._getCachedInventoryList() calls Resource_zGetInventoryCacheResult and Resource_zInsertInventoryCacheResult. The former should use R-R connection, thus the latter should also use R-R connection.

SimulationTool_zTrimInventoryCacheFromDateOnCatalog is called as a part of indexObject and it contains a subquery on stock table, thus it should use R-C connection but in the same Zope transaction of alternateReindexObject activity, _getCachedInventoryList is called where we use R-R connection.

If we use 2 connections and one contains DELETE operation, we get a deadlock.

/cc @vpelletier @jerome

I found difficulty to adapt erp5_stock_cache code to R-C + R-R combination.

Is this BT actually used somewhere ?

I have not had to think about it in many years, so my knowledge may be critically outdated. Last I heard about it, it had an unidentified bug affecting some reporting (this is so old memory that I do not have much details at all, sorry).

Back to your question:

I have the impression that the connector a ZSQLMethod uses can be influenced by an argument. Am I making that up ? If this is actually a thing, here are some ideas:

An idea I have is really not elegant: add a parameter to let the caller pick what connector to use, and propagate it down to the relevant ZSQLMethod. This feels dirty (an implementation detail finding its way up the call chain at a level we should probably not have to careabout it) and I can imagine it would cause a lot of diff.

Another direction could be to introduce some kind of inspection of the current execution environment. If what we care about is running in activities, then activity runtime environment could be a way to pass such value, but this is not really nice. Maybe a transaction-global context manager which would be set when indexing and would be checked at the low-level code (calling the ZSQLMethod).

Another question is how to automatically generate R-C connection while updating, to avoid an error of testUpgradeInstanceWithOldDataFs test, that will happen in reality for normal site as well.

  Module script, line 28, in Alarm_runUpgrader
   - <PythonScript at /erp5/Alarm_runUpgrader used for /erp5/portal_alarms/upgrader_check_upgrader>
   - Line 28
    portal.portal_templates.Base_postCheckConsistencyResult(**method_kw)
  Module AccessControl.ZopeGuards, line 500, in guarded_apply
    return builtin_guarded_apply(func, args, kws)
  Module AccessControl.ZopeGuards, line 523, in builtin_guarded_apply
    return func(*arglist, **argdict)
  Module Products.ERP5Type.patches.PythonScript, line 182, in __call__
    return self._orig_bindAndExec(args, kw, None)
  Module Shared.DC.Scripts.Bindings, line 372, in _bindAndExec
    return self._exec(bound_data, args, kw)
  Module Products.ERP5Type.tests.ERP5TypeTestCase, line 1614, in _exec
    return PythonScript_exec(self, *args)
  Module Products.PythonScripts.PythonScript, line 349, in _exec
    result = function(*args, **kw)
  Module script, line 9, in Base_postCheckConsistencyResult
   - <PythonScript at /erp5/Base_postCheckConsistencyResult used for /erp5/portal_templates>
   - Line 9
    fixit=fixit, filter=filter_dict,)
  Module Products.ERP5Type.Core.Folder, line 1472, in checkConsistency
    **kw
  Module Products.ERP5Type.Base, line 2698, in checkConsistency
    constraint_instance.fixConsistency)(self, **kw)
  Module Products.ERP5Type.UnrestrictedMethod, line 70, in unrestricted_apply
    return function(*args, **kw)
  Module Products.ERP5Type.mixin.constraint, line 107, in fixConsistency
    return self.checkConsistency(obj, fixit=1, **kw)
  Module Products.ERP5Type.mixin.constraint, line 94, in checkConsistency
    return self._checkConsistency(obj, fixit, **kw)
  Module erp5.component.document.erp5_version.ScriptConstraint, line 60, in _checkConsistency
    
  Module Products.ERP5Type.patches.PythonScript, line 182, in __call__
    return self._orig_bindAndExec(args, kw, None)
  Module Shared.DC.Scripts.Bindings, line 372, in _bindAndExec
    return self._exec(bound_data, args, kw)
  Module Products.ERP5Type.tests.ERP5TypeTestCase, line 1614, in _exec
    return PythonScript_exec(self, *args)
  Module Products.PythonScripts.PythonScript, line 349, in _exec
    result = function(*args, **kw)
  Module script, line 9, in TemplateTool_checkBusinessTemplateInstallation
   - <PythonScript at /erp5/TemplateTool_checkBusinessTemplateInstallation used for /erp5/portal_templates>
   - Line 9
    dry_run=(not fixit))
  Module Products.ERP5.Tool.TemplateTool, line 1487, in upgradeSite
    update_catalog=update_catalog)
  Module Products.ERP5.Tool.TemplateTool, line 1358, in updateBusinessTemplateFromUrl
    update_catalog=update_catalog)
  Module Products.ERP5Type.Base, line 259, in __call__
    result = self.__dict__['_m'](instance, *args, **kw)
  Module Products.ERP5.Document.BusinessTemplate, line 5555, in install
    return self._install(*args, **kw)
  Module Products.ERP5.Document.BusinessTemplate, line 5497, in _install
    trashbin=trashbin, installed_bt=installed_bt)
  Module Products.ERP5.Document.BusinessTemplate, line 1407, in install
   - __traceback_info__: portal_components/module.erp5.ExpandPolicy
    container.manage_delObjects([object_id])
  Module Products.ERP5Type.CopySupport, line 217, in manage_delObjects
    return ObjectManager.manage_delObjects(self, ids, REQUEST)
  Module OFS.ObjectManager, line 573, in manage_delObjects
    self._delObject(id)
  Module Products.ERP5Type.Core.Folder, line 1633, in _delObject
    object.manage_beforeDelete(object, self)
  Module Products.ERP5Type.CopySupport, line 341, in manage_beforeDelete
    self.unindexObject()
  Module Products.ERP5Type.CopySupport, line 375, in unindexObject
    catalog.beforeUnindexObject(None, path=self.getPath(), uid=uid)
  Module Products.ERP5Catalog.CatalogTool, line 1023, in beforeUnindexObject
    self.beforeUncatalogObject(path=path,uid=uid, sql_catalog_id=sql_catalog_id)
  Module Products.ZSQLCatalog.ZSQLCatalog, line 880, in beforeUncatalogObject
    catalog.beforeUncatalogObject(uid=uid,path=path)
  Module Products.ZSQLCatalog.SQLCatalog, line 1492, in beforeUncatalogObject
    method(uid=uid, path=path)
  Module Products.ERP5Type.patches.DA, line 194, in DA__call__
   - <SQL Method at /erp5/portal_catalog/erp5_mysql_innodb/z_delete_uid>
    c))
AttributeError: The database connection <em>erp5_sql_read_committed_connection</em> cannot be found.

The first thing which comes to mind is ERP5Generator.setupTools, which is called from .setup which may receive an update argument with a true value. I do not immediately remember when that is called though, I just see that ZMySQLDA objects are created there when missing.

Another direction could be to introduce some kind of inspection of the current execution environment.

Thanks, this reminded me of another idea I had. Let me close this merge request, that is now superceded by !1997.

closed