Repository navigation
bug: Skip partition REORGANIZE when an existing boundary already covers the day (#434) - #447
Merged
Merged
Conversation
…rs the day syslog_partition_create() decided a daily partition was missing purely by name, so a table from an older TO_DAYS schema or one left inconsistent by an interrupted 4.2-to-4.11 upgrade would repeatedly emit a REORGANIZE of dMaxValue that MySQL rejected with error 1493 (VALUES LESS THAN value must be strictly increasing), wedging recovery and parking writes in the dMaxValue safety partition. Compare the target boundary against the highest existing concrete boundary first and treat an already-covered day as a successful no-op. Fixes #434 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Recovery counts already-covered days against its creation budget, leaving needed partitions uncreated and repeatedly deferring retention.
1 open issue
What changed in this PR
This PR addresses the partition-creation error reported in #434 by checking existing boundaries before splitting dMaxValue, complementing the upgrade compatibility fixes in #439.
Changes:
- Skip partition creation when an existing
TO_DAYSor epoch boundary already covers the requested day. - Add tests for covered-day no-ops, lock release, and creation beyond the existing range.
- Document the fix in the development changelog.
| File | Description |
|---|---|
| tests/Unit/PartitionReorganizeCoverageTest.php | Adds boundary-aware partition creation regression tests. |
| includes/functions.php | Guards against REORGANIZE statements with already-covered boundaries. |
| CHANGELOG.md | Documents the partition recovery fix for #434. |
Review effort: Balanced
…do not consume the recovery budget Address PR review: a boundary-aware no-op in syslog_partition_create() still consumed the bounded per-run recovery budget because syslog_partition_check() identified missing days purely by partition name. A last partition named d20261111 whose upper boundary already reached Nov 15 made recovery on Nov 12 spend all three slots on already-covered days, starving the genuine Nov 15 gap and deferring retention on every run. syslog_partition_check() now compares the target boundary against the highest existing concrete boundary (mirroring syslog_partition_create()), so covered days no longer count as missing or consume creation slots, while genuine gaps are still created. When the partition expression or boundary metadata cannot be read it conservatively requests creation, and the create() guard keeps that safe. Adds a repeated-recovery regression test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…he boundary-aware partition check Adds two direct syslog_partition_check() cases so the changed-line coverage gate stays at 100%: one that resolves UNIX_TIMESTAMP coverage by boundary (covered vs genuine gap), and one where unreadable (non-numeric) boundary metadata falls back to requesting creation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
xmacan
approved these changes
Nov 13, 2026
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.




Description
syslog_partition_create()decided whether a daily partition was missing purely by name (e.g.d20261112). When that name was absent it unconditionally issued:A
REORGANIZEofdMaxValueonly succeeds when the new boundary is strictly greater than every existing concrete boundary. On a table carried over from an olderTO_DAYSschema — or one left inconsistent by an interrupted 4.2 → 4.11 upgrade — an existing partition can already reach or exceed that day's boundary under a different name. The name-only check never detected this, so theALTERwas rejected by MySQL with:Because
syslog_partition_recover()stops on the first create failure, recovery wedged permanently: every poller cycle re-emitted the doomedREORGANIZE, logged the same errors, and left new rows parked in thedMaxValuesafety partition while retention pruning stayed deferred.This change adds a boundary-aware guard. Before issuing the
REORGANIZE, it reads the highest existing concrete boundary (MAX(partition_description)) and the partition expression frominformation_schema.PARTITIONS, computes the target boundary in the table's native units (TO_DAYS(...)or the epoch literal), and if the day is already covered (target <= max existing boundary) treats it as a successful no-op instead of emitting DDL that cannot succeed. Genuinely new partitions (target beyond the current range) are still created exactly as before.Related Issue
Fixes #434 — "Cacti 1.2.3x Syslog 4.11. to 4.2 missing table & VALUES LESS THAN error on creation of partition". The user's log shows the exact failing statement and MySQL error 1493 on both
syslogandsyslog_removed, repeating every poller cycle.Motivation and Context
The missing-column error in that issue was already addressed by #439; the remaining, still-reproducing symptom is this partition-creation crash loop. The fix is scoped to the creation path where the DDL is emitted, so recovery no longer wedges regardless of how the legacy/partly-upgraded table reached its current partition layout.
How Has This Been Tested?
Ran the plugin's Pest suite against a Cacti checkout with the plugin at
plugins/syslog:Result: 647 passed (2695 assertions), with 1 pre-existing unrelated warning (
/proccmdline stub inProcessHelperCompatTest).Added
tests/Unit/PartitionReorganizeCoverageTest.phpwith three cases:TO_DAYSday → noREORGANIZEissued, returns success, lock released;UNIX_TIMESTAMPday → same;REORGANIZEis still issued.The existing partition-create/overflow/recovery tests continue to pass unchanged (the new
information_schemalookup is inert under their stubs, preserving current behavior).Types of changes
Checklist: