i'm a newbie with MySQL Galera ..and even i cannot remember the MariaDB equivalent name
but sounds safe, on code review

P.S
thanks for your 1st pr ?

from a pure SQL concept looks good. Could be condensed I think to a single query:

$db->setQuery('SELECT EXISTS (SELECT 1 FROM ' . $db->quoteName('#__finder_tokens' UNION ALL SELECT 1 FROM ' . $db->quoteName('#__finder_tokens_aggregate') . ')');

Alternately:

I"m not 100% sure of the Joomla structures however if doing individual tests before alter:

foreach (array($db->quoteName('#__finder_tokens), $db->quoteName('#__finder_tokens_aggregate')) as $tbl) {
            $db->setQuery('SELECT EXISTS (SELECT 1 FROM ' . $tbl . ')');
            if ($db->loadResult() === 0) {
                continue;
            }
            $db->setQuery('ALTER TABLE ' . $tbl . ' ENGINE = MEMORY');
            $db->execute();
} 

This pull request has been automatically rebased to 5.2-dev.

Unfortunately this is an issue which is VERY hard to test. Very few people have a MySQL cluster to test all of this and without 2 tests we wont move forward here. So I'd like to make a proposal: Could you maybe create a new PR with a new switch in the Smart Search configuration in the backend. That switch would enable the dynamic management of those tables and if switched off, it would basically disable Indexer::toggleTables(), allowing you to set the table to InnoDB once and them not being changed all the time or requiring core hacks to disable that method. Would that be acceptable?

In that case it would have to go into 5.3.

This pull request has been automatically rebased to 5.3-dev.

This pull request has been automatically rebased to 5.4-dev.

Hello there,

I have tested this code (just copied and pasted) in a joomla 6.1 with a MySQL Cluster 8.4.
Before this code I had errors indexing in the smart search component.

After using this code that worked.

If you test in a cluster /group replica its important to have ql_generate_invisible_primary_key activated so you can drop _finder_tokens and finder_tokens_aggregate tables to recreate with insivble pk for group replicas.

Without this code the tables engine is changed to memory even if you create it with innodb before.

Thanks

Hello there,

I have tested this code (just copied and pasted) in a joomla 6.1 with a MySQL Cluster 8.4. Before this code I had errors indexing in the smart search component.

After using this code that worked.

If you test in a cluster /group replica its important to have ql_generate_invisible_primary_key activated so you can drop _finder_tokens and finder_tokens_aggregate tables to recreate with insivble pk for group replicas.

Without this code the tables engine is changed to memory even if you create it with innodb before.

Thanks

@lscorcia If that means that you have successfully tested this PR, then please go to the PR in the issue tracker here https://issues.joomla.org/tracker/joomla-cms/42414 and use the blue "Test this" button, select your test result and submit. In this way your test will be properly counted. Each PR needs 2 successful human tests. Thanks in advance.

I have tested this item ✅ successfully on 0df1e8a

Tested in Joomla 5.4.

I have tested this item ✅ successfully on 0df1e8a

Tested in Joomla 6.1 with MySQL Cluster 8.4

I have tested this item ✅ successfully on 0df1e8aTested in Joomla 5.4.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/42414.

@lscorcia Sorry, my mistake. I wanted to ping @PepDevel .

I will remove your test as the author of a pull request is required to have tested his own PR anyway, so only tests from other people than the author count.

So we have one test from @PepDevel .

I have tested this item ✅ successfully on 0df1e8a

I tested by installing a MariaDB Galera cluster with one node and wsrep_mode DISALLOW_LOCAL_GTID by hacking JBT:

Prepare

• In joomla-branches-tester git-cloned folder put the files compose.galera.yml and galera.patch
• Run scripts/create create 54 (if you not have already 5.4-dev instance running)
• Patch JBT with
  • docker compose -f compose.galera.yml up -d
  • git apply galera.patch

Test before PR

• Install Joomla with scripts/database 54 galera
• docker exec -it jbt-galera bash -c "mariadb -uroot -proot test_joomla_54"
  • You'll see all tables are InnoDB unless ajbt54_finder_tokens and ajbt54_finder_tokens_aggregate are MEMORY
    • select TABLE_NAME, ENGINE from information_schema.TABLES where TABLE_SCHEMA = 'test_joomla_54';
  • Change the two tables to InnoDB as well:
    • ALTER TABLE ajbt54_finder_tokens ENGINE=InnoDB;
    • ALTER TABLE ajbt54_finder_tokens_aggregate ENGINE=InnoDB;
  • You can verify again and all tables are now InnoDB
• If you create an article in the backend the two tables are set back to MEMORY
• And if you try to create second article you are getting red: Save failed with the following error: Galera replication not supported

Apply PR and test

• docker exec jbt-54 bash -lc "gh pr checkout 42414"
• Install Joomla again scripts/database 54 galera
• Change the two tables to InnoDB again
• ✅ created two articles and running Smart Search Index via backend without errors
• ✅ checked all tables are still InnoDB

Verify normal MySQL with MySQLi driver is still working

• scripts/database 54 mysqli
• ✅ Created two articles and running Smart Search Index
• ✅ Checked the two tables are still MEMORY
  • docker exec -it jbt-mysql mysql -uroot -proot test_joomla_54;

Clean-Up

• Either simple drop whole JBT installation
  • scripts/clean
  • cd ..
  • rm -rf joomla-branches-tester
• Or running
  • docker compose -f compose.galera.yml down
  • git apply -R galera.patch
  • rm compose.galera.yml galera.patch

I have tested this item ✅ successfully on 0df1e8a

I tested by installing a MariaDB Galera cluster with one node and wsrep_mode DISALLOW_LOCAL_GTID by hacking JBT:

Prepare

• In joomla-branches-tester git-cloned folder put the files compose.galera.yml and galera.patch
• Run scripts/create create 54 (if you not have already 5.4-dev instance running)
• Patch JBT with
  • docker compose -f compose.galera.yml up -d
  • git apply galera.patch

Test before PR

• Install Joomla with scripts/database 54 galera
• docker exec -it jbt-galera bash -c "mariadb -uroot -proot test_joomla_54"
  • You'll see all tables are InnoDB unless ajbt54_finder_tokens and ajbt54_finder_tokens_aggregate are MEMORY
    • select TABLE_NAME, ENGINE from information_schema.TABLES where TABLE_SCHEMA = 'test_joomla_54';
  • Change the two tables to InnoDB as well:
    • ALTER TABLE ajbt54_finder_tokens ENGINE=InnoDB;
    • ALTER TABLE ajbt54_finder_tokens_aggregate ENGINE=InnoDB;
  • You can verify again and all tables are now InnoDB
• If you create an article in the backend the two tables are set back to MEMORY
• And if you try to create second article you are getting red: Save failed with the following error: Galera replication not supported

Apply PR and test

• docker exec jbt-54 bash -lc "gh pr checkout 42414"
• Install Joomla again scripts/database 54 galera
• Change the two tables to InnoDB again
• ✅ created two articles and running Smart Search Index via backend without errors
• ✅ checked all tables are still InnoDB

Verify normal MySQL with MySQLi driver is still working

• scripts/database 54 mysqli
• ✅ Created two articles and running Smart Search Index
• ✅ Checked the two tables are still MEMORY
  • docker exec -it jbt-mysql mysql -uroot -proot test_joomla_54;

Clean-Up

• Either simple drop whole JBT installation
  • scripts/clean
  • cd ..
  • rm -rf joomla-branches-tester
• Or running
  • docker compose -f compose.galera.yml down
  • git apply -R galera.patch
  • rm compose.galera.yml galera.patch

I have tested this item ✅ successfully on 0df1e8a

I tested by installing a MariaDB Galera cluster with one node and wsrep_mode DISALLOW_LOCAL_GTID by hacking JBT:

Prepare

• In joomla-branches-tester git-cloned folder put the files compose.galera.yml and galera.patch
• Run scripts/create create 54 (if you not have already 5.4-dev instance running)
• Patch JBT with
  • docker compose -f compose.galera.yml up -d
  • git apply galera.patch

Test before PR

• Install Joomla with scripts/database 54 galera
• docker exec -it jbt-galera bash -c "mariadb -uroot -proot test_joomla_54"
  • You'll see all tables are InnoDB unless ajbt54_finder_tokens and ajbt54_finder_tokens_aggregate are MEMORY
    • select TABLE_NAME, ENGINE from information_schema.TABLES where TABLE_SCHEMA = 'test_joomla_54';
  • Change the two tables to InnoDB as well:
    • ALTER TABLE ajbt54_finder_tokens ENGINE=InnoDB;
    • ALTER TABLE ajbt54_finder_tokens_aggregate ENGINE=InnoDB;
  • You can verify again and all tables are now InnoDB
• If you create an article in the backend the two tables are set back to MEMORY
• And if you try to create second article you are getting red: Save failed with the following error: Galera replication not supported

Apply PR and test

• docker exec jbt-54 bash -lc "gh pr checkout 42414"
• Install Joomla again scripts/database 54 galera
• Change the two tables to InnoDB again
• ✅ created two articles and running Smart Search Index via backend without errors
• ✅ checked all tables are still InnoDB

Verify normal MySQL with MySQLi driver is still working

• scripts/database 54 mysqli
• ✅ Created two articles and running Smart Search Index
• ✅ Checked the two tables are still MEMORY
  • docker exec -it jbt-mysql mysql -uroot -proot test_joomla_54;

Clean-Up

• Either simple drop whole JBT installation
  • scripts/clean
  • cd ..
  • rm -rf joomla-branches-tester
• Or running
  • docker compose -f compose.galera.yml down
  • git apply -R galera.patch
  • rm compose.galera.yml galera.patch

RTC

RTC

I have requested a review. Once we have one review, this PR can be merged from my point of view.

@lscorcia I have doubts if @muhme 's test covers all scenarios.

In the code I can see that the indexer sets the table to InnoDB when the memory limit will be reached. This happens at 3 places:

• https://github.com/lscorcia/joomla-cms/blob/c9467e75941c86017cd56d20e67a5264725084ef/administrator/components/com_finder/src/Indexer/Indexer.php#L421
• https://github.com/lscorcia/joomla-cms/blob/c9467e75941c86017cd56d20e67a5264725084ef/administrator/components/com_finder/src/Indexer/Indexer.php#L441
• https://github.com/lscorcia/joomla-cms/blob/c9467e75941c86017cd56d20e67a5264725084ef/administrator/components/com_finder/src/Indexer/Indexer.php#L958

But there is only one place where it is trying to set it back to Memory, and that is exactly after the truncateTable statements here: https://github.com/lscorcia/joomla-cms/blob/c9467e75941c86017cd56d20e67a5264725084ef/administrator/components/com_finder/src/Indexer/Indexer.php#L621-L628

But for empty tables it will never happen at that place with your PR.

So if I am not wrong, when you are on a normal MySQL database, not a Galera cluster, and the memory limit is reached on indexing, you will never come back to Memory.

Or am I missing something?

It would need a test with a small PHP memory limit and enough indexer tokens.