Logging table schemas should be updated when extension table schemas are updated

I just ran into an issue while enabling Geocoder on a relatively fresh Civi install. install.sql generated a table, then hook_civicrm_managed tried adding all those record in .mgd.php, which failed because log_civicrm_geocoder didn't exist.

I know this isn't a simple lift - but given the recent conversations that civix might phase out the generation of .sql files, this seems like the opportune moment to raise the issue.

There was #1618 (closed), but is maybe a different timing issue. To be honest I wasn't sure it actually was solved there (https://github.com/civicrm/civicrm-core/pull/21135#issuecomment-910278031), but this might be different.

added git:civicrm-core sig:bug triaged labels

I agree that this is basically #1618 (closed) - but I also don't think https://github.com/civicrm/civicrm-core/pull/21135 is a complete solution, for the reasons you mention @DaveD.

Pinging @seamuslee since 21135 is yours - I think that to properly fix this problem, the upgradeLogTables() method you add in 21135 should be called between every upgrade step. I don't know what the performance implications of this are though.

A lighter option that would solve the problem on Geocoder but maybe not all cases - the CRM_X_Upgrade_Base class that is created by civix generate:upgrader could run upgradeLogTable() as part of executeSqlFile(). Is that possible to implement with mixins now, or would we just have to fix this on a going-forward basis?

Kind of related: #2237, in the sense that communication between core and extensions isn't automatic regarding their tables, and there's some discussion of an upgrade mixin.

upgradeLogTables() method ... should be called between every upgrade step. I don't know what the performance implications of this are though.

I think that would be slow - reconciliation seems to be a heavy step.

Does logging need to be on during upgrades? Does anyone revert a failed upgrade by manually recreating records from the logging tables - they'll just restore a backup, right? Could the extension upgrader turn off logging at the start, then turn it back on at the end?

@DaveD Normally when folks mention "reconciliation" they're referencing managed entity reconciliation, which is quite heavy.

Reconciling log tables isn't that heavy in my experience, because the data is gathered from the INFORMATION_SCHEMA.COLUMNS table, and it's just an array comparison to find differences. That said, I don't know how that scales, but I'm also guessing it could be optimized considerably.

Turning off logging during an upgrade is also a viable strategy. I normally don't take my db offline for extension upgrades, but I suppose I could start.

A quick test:

cv ev --user=admin "$s = microtime(TRUE); $x = new CRM_Logging_Schema(); $x->fixSchemaDifferences(); echo microtime(TRUE) - $s;"

23.655997037888

This is where I've already primed the system so a plain cv ev call takes 0.0001 seconds.

Hi,

Thank you for raising an issue to help improve CiviCRM. As you may know, this issue has not had any activity for quite some time, so we have closed it.

We would like you to help us to determine if this issue should be re-opened:

• If this issue was reporting a bug, can you attempt to reproduce it on a latest version of CiviCRM?
• If this issue was proposing a new feature, can you verify if the feature proposal is still relevant? Did it get the concept-approved label? Have other people also shown interest? Could it be implemented as an extension?

If the answer to either question is yes, please feel free to comment or re-open the issue. Please also consider:

• Is it something that you could help implement, either by sending a patch or hiring someone who can?
• Would you be willing to fund the work, either through a CiviCRM partner, or the Core Team?

Thank you for your help and contributions to CiviCRM.

P.S. This is an automated message, see infra/gitlab issue 20. We understand that automatic responses are annoying, but given the number of open issues as the project evolves, we need a bit of help to triage and prioritise the most relevant issues.

closed