Skip to content
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

zdb: dump ZAP_FLAG_UINT64_KEY ZAPs properly #16334

Merged
merged 1 commit into from
Jul 17, 2024

Conversation

robn
Copy link
Member

@robn robn commented Jul 9, 2024

[Sponsors: Klara, Inc., Wasabi Technology, Inc.]

Motivation and Context

I was using zdb to look around my local pool, and it kept crashing on this very strange ZAP that made no sense to me. Turns out it was the BRT ZAP, which has binary keys rather than text keys, and zdb had no idea what to make of that.

Description

Detect ZAP_FLAG_UINT64_KEY ZAPs in dump_zap(). If seen, synthesize a text "key" from the first uint64, and show the value using the same "array of integers" code used elsewhere.

This is not especially useful for keys longer than one uint64 (eg DDT keys, which are 40 bytes), but that's sorta difficult to make a generic display for and zdb already has structured formatters for dedup and block reference tables, so not super useful anyway. Really, I just wanted something useful and not-crashy.

How Has This Been Tested?

Hand tested on Linux and FreeBSD.

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:

These are used for DDT and BRT stores. There's limited information
available to produce meaningful output, but at least we can put
something on screen rather than crashing.

Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Signed-off-by: Rob Norris <rob.norris@klarasystems.com>
Copy link
Member

@amotin amotin left a comment

Choose a reason for hiding this comment

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

I saw this also, but was thinking towards extending ZAP API to allow key length reporting, after which we could to it properly and universally, but also haven't got to it yet.

@robn
Copy link
Member Author

robn commented Jul 10, 2024

Yeah, I'd like to do that. Seems like the _uint64 variants might not even be necessary if it can automatically decide what to do from the the flag. That's be really nice for zap_lookup inside a cursor especially.

Alas, not really on my radar at the moment, but I've chucked it on the list.

@robn robn mentioned this pull request Jul 17, 2024
13 tasks
@tonyhutter tonyhutter merged commit dc91e74 into openzfs:master Jul 17, 2024
21 of 25 checks passed
robn added a commit to robn/zfs that referenced this pull request Jul 18, 2024
These are used for DDT and BRT stores. There's limited information
available to produce meaningful output, but at least we can put
something on screen rather than crashing.

Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.

Signed-off-by: Rob Norris <rob.norris@klarasystems.com>
Reviewed-by: Alexander Motin <mav@FreeBSD.org>
Reviewed-by: Tony Hutter <hutter2@llnl.gov>
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Sep 4, 2024
These are used for DDT and BRT stores. There's limited information
available to produce meaningful output, but at least we can put
something on screen rather than crashing.

Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.

Signed-off-by: Rob Norris <rob.norris@klarasystems.com>
Reviewed-by: Alexander Motin <mav@FreeBSD.org>
Reviewed-by: Tony Hutter <hutter2@llnl.gov>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants