Skip to content

ZTS: mmp_on_uberblocks.ksh simplify#18452

Open
behlendorf wants to merge 1 commit intoopenzfs:masterfrom
behlendorf:zts-mmp_on_uberblocks
Open

ZTS: mmp_on_uberblocks.ksh simplify#18452
behlendorf wants to merge 1 commit intoopenzfs:masterfrom
behlendorf:zts-mmp_on_uberblocks

Conversation

@behlendorf
Copy link
Copy Markdown
Contributor

Motivation and Context

https://github.com/openzfs/zfs/actions/runs/24792352037/job/72552994804?pr=18398

Resolve a long standing flaw in the mmp_on_uberblocks.ksh test case which was causing CI failures.

Description

The last portion of mmp_on_uberblocks.ksh was intended to verify that the sequence number was incremented. However, it failed to account for the case where a txg sync would occur resulting in the sequence number being correctly reset.

Rather than add additional code to detect this that check has been removed. The mmp update frequency is still verified via the kstat which is a more reliably mechanism to detect the writes. There are several other mmp tests which verify the uberblock changes are reflected on disk so there's no significant loss of test coverage.

Finally, the test case has been simplified to use the within_percent function for readability.

How Has This Been Tested?

Locally tested with 100 iterations of the updated test and will be further verified by the CI.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Quality assurance (non-breaking change which makes the code more robust against bugs)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Copilot AI review requested due to automatic review settings April 22, 2026 22:37
@behlendorf behlendorf added Component: Test Suite Indicates an issue with the test framework or a test case Status: Code Review Needed Ready for review and testing labels Apr 22, 2026
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR simplifies the mmp_on_uberblocks.ksh ZTS test to avoid a known flaky sequence-number assertion and instead validate MMP write frequency via kstat, using within_percent for readability.

Changes:

  • Removes the mmp_seq increment check which could fail when a TXG sync resets the sequence.
  • Computes an expected MMP write target from the current MULTIHOST_INTERVAL and verifies writes against that target.
  • Simplifies the pass/fail logic into a single within_percent check.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/zfs-tests/tests/functional/mmp/mmp_on_uberblocks.ksh Outdated
Comment thread tests/zfs-tests/tests/functional/mmp/mmp_on_uberblocks.ksh
The last portion of mmp_on_uberblocks.ksh was intended to verify that
the sequence number was incremented.  However, it failed to account for
the case where a txg sync would occur resulting in the sequence number
being correctly reset.

Rather than add additional code to detect this that check has been
removed.  The mmp update frequency is still verified via the kstat
which is a more reliably mechanism to detect the writes.  There are
several other mmp tests which verify the uberblock changes are reflected
on disk so there's no significant loss of test coverage.

Finally, the test case has been simplified to use the within_percent
function for readability.

Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
@behlendorf behlendorf force-pushed the zts-mmp_on_uberblocks branch from 0b466b8 to d6206f9 Compare April 22, 2026 22:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component: Test Suite Indicates an issue with the test framework or a test case Status: Code Review Needed Ready for review and testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants