put rexpiring delay logic in Lock class #15874
Conversation
diosmosis
added
not-in-changelog
|
looks good if tests pass @diosmosis I haven't done a full test but debugged and looks good. It was applying the correct TTL and basically the lock instance now persists the time across individual archives as the instance is created when the archives starts and added to the stack. It seems when browser archiving is enabled, it may not 100% work though. In my case it was creating the lock eg every time for each metric I reckon this impacts only browser archiving though (haven't tested cron archiving). Maybe you could quickly double check it only creates the lock once per cron archive? |
|
Actually, I think it might have been because I had archiving forced ( |
|
@tsteur I can confirm it doesn't get called in core:archive for me, but for it to get called, the archiving job would have to run for a long time (even if I set the timeout to 30s, it would have to run for more than my local jobs run for). |
core/DataAccess/LogAggregator.php
Outdated
| /** | ||
| * @var \Piwik\Tracker\Db|\Piwik\Db\AdapterInterface|\Piwik\Db | ||
| */ | ||
| private $readerDb; |
There was a problem hiding this comment.
btw @diosmosis I suppose this one isn't needed?
|
@diosmosis sorry seems one test is now failing: https://travis-ci.org/github/matomo-org/matomo/jobs/680785282#L923 Not sure if the value is expected? Feel free to merge once the test is fixed. |
* Only use one reader db instance per LogAggregator. * use re-expire logic in Lock itself * tweak * Remove unneeded var * fix test
* Only use one reader db instance per LogAggregator. * use re-expire logic in Lock itself * tweak * Remove unneeded var * fix test
Fixes #15870