fix(tracker): make cleanup cron observable + robust (#2) - #9
Merged
Conversation
cleanupOldRecords only logged on success, so a job that never fired or failed early left no trace and old synced rows accumulated unnoticed. - Log at entry (cutoff included) so every run is visible in datasync.log, per the issue's action item to make execution observable. - Log failures to datasync.log too, not just the exception log, so 'cleanup not working' is diagnosable from one file. - Match created_at as a fallback when synced_at IS NULL: a synced row should always carry synced_at, but any path that set sync_completed=1 without it would never match (NULL < date is never true) and the rows would accumulate forever. Source-side OpenMage/Magento 1 module, so Varien_*/Zend_Log are correct here. Fixes #2
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2.
What
The source-module
cleanupOldRecords()cron job (maho_datasynctracker_cleanup, runs 0 3 * * 0) was reported as not firing / failing silently, with old synced rows accumulating.The plumbing is all present and correct (cron wired in
config.xml, method exists,synced_atcolumn exists and is set by the destination'smarkAsSynced()). The real gap is observability plus one latent data edge case:datasync.logeven when it deletes 0 rows or errors before completing. Previously the only log line was on success-exit, so a job that never ran or failed early left no trace, which is exactly the reported symptom.datasync.logtoo (not just the exception log), so "cleanup isn't working" is diagnosable from a single file.created_atfallback whensynced_at IS NULL: a synced row should always carrysynced_at, but any path that setsync_completed = 1without it would never match the delete (NULL < dateis never true) and would accumulate forever. The delete now also catches those.Why not a code-presence fix
This issue was originally filed against an assumption the cron/method might be missing. They aren't, so this hardens what exists rather than re-adding it. The remaining "is the host cron actually running this job" check is operational (verify
cron_scheduleformaho_datasynctracker_cleanupon the live store) and can't be fixed in code; the entry-log added here makes that verifiable.Notes
Varien_*/Zend_Logare intentional here (this is not Maho).php -lclean.Test plan
DataSyncTracker cleanup: starting...line appears invar/log/datasync.log, followed by adeleted N old synced recordsline.cron_schedulehasmaho_datasynctracker_cleanuprows transitioning tosuccess.sync_completed=1, synced_at=NULL, created_at < 30d agoand confirm it is now removed.