Skip to content

zed: Add synchronous zedlets #17335

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

tonyhutter
Copy link
Contributor

@tonyhutter tonyhutter commented May 14, 2025

Motivation and Context

Allow zedlets to execute synchronously.

Description

Historically, ZED has blindly spawned off zedlets in parallel and never worried about their completion order. This means that you can potentially have zedlets for event number 2 starting before zedlets for event number 1 had finished. Most of the time this is fine, and it actually helps a lot when the system is getting spammed with hundreds of events.

However, there are times when you want your zedlets to be executed in sequence with the event ID. That is where synchronous zedlets come in.

ZED will wait for all previously spawned zedlets to finish before running a synchronous zedlet. Synchronous zedlets are guaranteed to be the only zedlet running. No other zedlets may run in parallel with a synchronous zedlet. Users should be careful to only use synchronous zedlets when needed, since they decrease parallelism.

To make a zedlet synchronous, simply add a "-sync-" immediately following the event name in the zedlet's file name:

	EVENT_NAME-sync-ZEDLETNAME.sh

For example, if you wanted a synchronous statechange script:

	statechange-sync-myzedlet.sh

How Has This Been Tested?

Test case added

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)
  • 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)

Checklist:

Copy link
Contributor

@behlendorf behlendorf left a comment

Choose a reason for hiding this comment

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

Looks good, nice and easy to use. Are there any existing zedlets we ship which should be made synchronous as part of this change?

@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label May 15, 2025
@tonyhutter
Copy link
Contributor Author

@behlendorf I updated this with your changes, including a scrub_finish check.

@tonyhutter
Copy link
Contributor Author

Are there any existing zedlets we ship which should be made synchronous as part of this change?

Probably these:

deadman-slot_off.sh
statechange-slot_off.sh

...potentially the LED scripts too:

statechange-led.sh
vdev_clear-led.sh
vdev_attach-led.sh
pool_import-led.sh

However, the LED scripts can take in the milliseconds to seconds range to change the LED value, so I can understand keeping them async.

Historically, ZED has blindly spawned off zedlets in parallel and never
worried about their completion order.  This means that you can
potentially have zedlets for event number 2 starting before zedlets for
event number 1 had finished.  Most of the time this is fine, and it
actually helps a lot when the system is getting spammed with hundreds
of events.

However, there are times when you want your zedlets to be executed
in sequence with the event ID.  That is where synchronous zedlets
come in.

ZED will wait for all previously spawned zedlets to finish before
running a synchronous zedlet.  Synchronous zedlets are guaranteed to be
the only zedlet running.  No other zedlets may run in parallel with a
synchronous zedlet.  Users should be careful to only use synchronous
zedlets when needed, since they decrease parallelism.

To make a zedlet synchronous, simply add a "-sync-" immediately
following the event name in the zedlet's file name:

	EVENT_NAME-sync-ZEDLETNAME.sh

For example, if you wanted a synchronous statechange script:

	statechange-sync-myzedlet.sh

Signed-off-by: Tony Hutter <[email protected]>
@tonyhutter
Copy link
Contributor Author

I ended up marking all those zedlets I mentioned earlier as synchronous in my latest push.

@tonyhutter
Copy link
Contributor Author

It was mentioned at the OpenZFS meeting today that we should add in timeouts for the synchronous zedlets. Let me look into that...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Code Review Needed Ready for review and testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants