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

Move queued tracking lock implementation to core for other uses. #14686

Merged
merged 5 commits into from Jul 29, 2019

Conversation

diosmosis
Copy link
Member

No description provided.


$tableName = self::getTableName();

DbHelper::createTable($tableName, "
Copy link
Member

Choose a reason for hiding this comment

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

As the table is installed on demand... If there's a race condition, should we maybe catch a "table already exists" error and fail silently? Or we just install the table the regular way and provide an update for it as well? IT be fine if table is not needed by most users.

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought the SQL was 'CREATE IF NOT EXISTS', but I'll double check.

Copy link
Member Author

Choose a reason for hiding this comment

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

Looks like it's not CREATE IF NOT EXISTS, but the exception is already caught: https://github.com/matomo-org/matomo/blob/3.x-dev/core/Db/Schema/Mysql.php#L452

use Piwik\Db;
use Piwik\DbHelper;

class MySqlLockBackend implements LockBackend
Copy link
Member

Choose a reason for hiding this comment

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

Might be good to write some simple tests for this backend as well? Can be probably copied from Queued tracking

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah yes, I forgot about the backend, I'll add the tests.

@mattab mattab modified the milestones: 3.11.0, 3.12.0 Jul 23, 2019
@diosmosis
Copy link
Member Author

@tsteur Updated, though can't be merged just yet. I moved the table creation to Schema.php, since the lock could be used during tracking, and Db::exec tries to use the profiler.

@diosmosis diosmosis merged commit 521eb11 into 3.x-dev Jul 29, 2019
@diosmosis diosmosis deleted the core-lock branch July 29, 2019 01:45
@mattab mattab added the c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself. label Oct 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants