Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clearTempTables: fix condition for user_job check #26696

Merged
merged 1 commit into from
Jul 3, 2023

Conversation

mlutfy
Copy link
Member

@mlutfy mlutfy commented Jul 2, 2023

Overview

Temporary SQL tables, such as civicrm_tmp_d_export_% , we not being cleaned up when the caches were flushed.

Technical Details

I was debugging a site that had over 200 tmp tables, which led me to this code. It wasn't a timestamp issue and the civicrm_user_job table was empty.

> SELECT TABLE_NAME as tableName, CREATE_TIME       FROM   INFORMATION_SCHEMA.TABLES       WHERE  TABLE_SCHEMA = DATABASE()       AND TABLE_NAME LIKE 'civicrm_tmp_d%';                       
+-------------------------------------------------------+---------------------+                                                                                                                                      
| tableName                                             | CREATE_TIME         |                                                                                                                                      
+-------------------------------------------------------+---------------------+                                                                                                                                      
| civicrm_tmp_d_export_54412d3d48f16067f976fde8045960f2 | 2023-04-05 17:16:55 |                                                                                                                                      
| civicrm_tmp_d_export_8657f02803e9a0a8e419cfda24c9d6a2 | 2023-04-05 17:16:56 |                                                                                                                                      
| civicrm_tmp_d_export_a327981d20dce5af941d818338a5e0d5 | 2023-05-16 07:55:13 |                                                                                                                                      
| civicrm_tmp_d_export_952662b95d62a4cc9f5af32c74cb858d | 2023-04-05 17:16:57 |                                                                                                                                      
| civicrm_tmp_d_export_44a687d0db471ff1bc3e4e484531a2e1 | 2023-06-30 11:28:17 |                                                                                                                                      
| civicrm_tmp_d_export_060caeb4e34f3eed293be3887f0a3af6 | 2023-04-05 17:16:54 |                                                                                                                                      
| civicrm_tmp_d_export_053b3f66b650568532615eca8b45b669 | 2023-07-01 18:36:10 |                                                                                                                                      
| civicrm_tmp_d_export_f38aa261fe164ce6b65ec9c1ab68935a | 2023-04-05 17:16:58 |                                                                                                                                      
| civicrm_tmp_d_export_24a5e88937df4312581b302befc90813 | 2023-06-20 08:36:10 |                                                                                                                                      
| civicrm_tmp_d_export_6311f86bf0554249293654227a6941ae | 2023-05-25 10:29:07 |                                                                                                                                      
[...]

@civibot
Copy link

civibot bot commented Jul 2, 2023

(Standard links)

@civibot civibot bot added the master label Jul 2, 2023
@@ -388,7 +388,7 @@ public static function clearTempTables($timeInterval = FALSE): void {
// TODO: Circa v5.60+, consider a more precise cleanup. Discussion: https://github.com/civicrm/civicrm-core/pull/24538
// A separate process will reap the UserJobs but here the goal is just not to delete them during cache clearing
// if they are still referenced.
if (!CRM_Core_DAO::executeQuery("SELECT count(*) FROM civicrm_user_job WHERE metadata LIKE '%" . $tableDAO->tableName . "%'")) {
if (!CRM_Core_DAO::singleValueQuery("SELECT count(*) FROM civicrm_user_job WHERE metadata LIKE '%" . $tableDAO->tableName . "%'")) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$tableDAO is being used here outside the loop. So it seems like multiple bugs here since this would only check the last one. I think this should be inside the loop:

while ($tableDAO->fetch()) {
  if (!CRM_Core_DAO::singleValueQuery("SELECT count(*) FROM civicrm_user_job WHERE metadata LIKE '%" . $tableDAO->tableName . "%'")) {
    $tables[] = $tableDAO->tableName;
  }
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@demeritcowboy good catch - I update the PR

@mlutfy mlutfy force-pushed the cleanupTmpTables branch from 040fc0d to e97bcba Compare July 2, 2023 18:51
@demeritcowboy demeritcowboy merged commit 4c983af into civicrm:master Jul 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants