Skip to content

unit/zap: uint64 keys#18639

Open
chrislongros wants to merge 1 commit into
openzfs:masterfrom
chrislongros:unit/zap-uint64-keys
Open

unit/zap: uint64 keys#18639
chrislongros wants to merge 1 commit into
openzfs:masterfrom
chrislongros:unit/zap-uint64-keys

Conversation

@chrislongros

@chrislongros chrislongros commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Motivation and Context

ZAP unit test that adds coverage for binary uint64-array keys (ZAP_FLAG_UINT64_KEY), as used by the dedup table (DDT) and the block reference table (BRT).

Description

Adds test_zap_uint64_keys. It creates a ZAP_FLAG_UINT64_KEY ZAP (always a fatzap) and runs dnode operations:

  • add then lookup — add a value and then look it up based on the key.
  • length — reports the value's size.
  • lookup_length — returns the value and its count.
  • update — replaces the value for an existing key.
  • remove — after which the key is no longer found.

How Has This Been Tested?

Built tests/unit/test_zap and ran the suite:

>   make unit T=zap

Running test suite with seed 0x8d74dd63...
zap.mock_microzap_sanity             [ OK    ] [ 0.00000743 / 0.00000665 CPU ]
zap.mock_fatzap_sanity               [ OK    ] [ 0.00003343 / 0.00003276 CPU ]
zap.zap_basic
  type=micro                         [ OK    ] [ 0.00001833 / 0.00001798 CPU ]
  type=fat                           [ OK    ] [ 0.00004697 / 0.00004631 CPU ]
zap.zap_add
  type=micro                         [ OK    ] [ 0.00001692 / 0.00001684 CPU ]
  type=fat                           [ OK    ] [ 0.00002890 / 0.00002889 CPU ]
zap.zap_update
  type=micro                         [ OK    ] [ 0.00001720 / 0.00001672 CPU ]
  type=fat                           [ OK    ] [ 0.00002462 / 0.00002462 CPU ]
zap.zap_remove
  type=micro                         [ OK    ] [ 0.00001514 / 0.00001478 CPU ]
  type=fat                           [ OK    ] [ 0.00002335 / 0.00002295 CPU ]
zap.zap_count
  type=micro                         [ OK    ] [ 0.00001221 / 0.00001223 CPU ]
  type=fat                           [ OK    ] [ 0.00001342 / 0.00001340 CPU ]
zap.zap_contains
  type=micro                         [ OK    ] [ 0.00000910 / 0.00000910 CPU ]
  type=fat                           [ OK    ] [ 0.00001333 / 0.00001333 CPU ]
zap.zap_length
  type=micro                         [ OK    ] [ 0.00000675 / 0.00000675 CPU ]
  type=fat                           [ OK    ] [ 0.00001119 / 0.00001120 CPU ]
zap.zap_increment
  type=micro                         [ OK    ] [ 0.00000781 / 0.00000782 CPU ]
  type=fat                           [ OK    ] [ 0.00001350 / 0.00001349 CPU ]
zap.zap_int
  type=micro                         [ OK    ] [ 0.00001516 / 0.00001512 CPU ]
  type=fat                           [ OK    ] [ 0.00001482 / 0.00001482 CPU ]
zap.zap_int_keys
  type=micro                         [ OK    ] [ 0.00000933 / 0.00000934 CPU ]
  type=fat                           [ OK    ] [ 0.00001411 / 0.00001411 CPU ]
zap.microzap_stats                   [ OK    ] [ 0.00001393 / 0.00001355 CPU ]
zap.fatzap_stats                     [ OK    ] [ 0.00001486 / 0.00001486 CPU ]
zap.uint64_keys                      [ OK    ] [ 0.00002066 / 0.00002039 CPU ]
zap.cursor
  type=micro                         [ OK    ] [ 0.00002114 / 0.00002087 CPU ]
  type=fat                           [ OK    ] [ 0.00002527 / 0.00002520 CPU ]
zap.cursor_serialize
  type=micro                         [ OK    ] [ 0.00001880 / 0.00001866 CPU ]
  type=fat                           [ OK    ] [ 0.00002643 / 0.00002614 CPU ]
zap.cursor_release_unused
  type=micro                         [ OK    ] [ 0.00001209 / 0.00001209 CPU ]
  type=fat                           [ OK    ] [ 0.00001635 / 0.00001633 CPU ]
zap.cursor_release_advance
  type=micro                         [ OK    ] [ 0.00000973 / 0.00000968 CPU ]
  type=fat                           [ OK    ] [ 0.00001369 / 0.00001364 CPU ]
zap.cursor_release_empty
  type=micro                         [ OK    ] [ 0.00000623 / 0.00000622 CPU ]
  type=fat                           [ OK    ] [ 0.00001187 / 0.00001188 CPU ]
zap.cursor_release_one
  type=micro                         [ OK    ] [ 0.00000896 / 0.00000897 CPU ]
  type=fat                           [ OK    ] [ 0.00001837 / 0.00001836 CPU ]
zap.zap_value_search
  type=micro                         [ OK    ] [ 0.00000798 / 0.00000799 CPU ]
  type=fat                           [ OK    ] [ 0.00001460 / 0.00001461 CPU ]
zap.zap_value_search_mask
  type=micro                         [ OK    ] [ 0.00000843 / 0.00000841 CPU ]
  type=fat                           [ OK    ] [ 0.00002130 / 0.00002130 CPU ]
41 of 41 (100%) tests successful, 0 (0%) test skipped.

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)
  • Tests (a change to the test suite that does not modify production code)

Checklist:

Add coverage for binary uint64-array keys (ZAP_FLAG_UINT64_KEY), the key
type used by the dedup table and block reference table.  Covers the
by-dnode operations on such a ZAP: add, lookup, length, lookup_length,
update and remove.

Signed-off-by: Christos Longros <chris.longros@gmail.com>
@robn robn self-requested a review June 6, 2026 22:52
@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Jun 8, 2026

@behlendorf behlendorf left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm all for the test coverage, let's just sync up with @robn on this one too.

@robn

robn commented Jun 9, 2026

Copy link
Copy Markdown
Member

Hey @chrislongros! Sorry for the delay getting back to you; we had a national holiday here on Monday and today I've been running errands.

So I'm excited to see you diving right in, and also slightly cranky that you beat me to the punch on the really cool stuff ;) I jest, this is great, and I'm extremely pleased to see this PR, and the others!

So I did already have a uint64 test in the pipeline, in a bigger bundle of FatZAP-specific tests. I hadn't felt great about it being there, since those are more aimed at internal stuff (eg leaf splits), so this is alright.

Here's my version of the uint64 tests: robn@3bc2666

I think I'm covering a bit more, but I've been looking mostly at API coverage, not specific use cases like DDT/BRT, which may or may not be a good thing. The main thing this one was waiting for me to do was check coverage and make sure I hadn't missed any particularly interested paths, and decide on whether the output overwriting behaviour of zap_lookup_length_uint64_by_dnode() == EOVERFLOW is part of the API or just incidental (leaning towards "incidental", but then I would want to go and fix that behaviour and add a test for that, so y'know, always something to do).

Let me know what you'd like to do here. We can ship yours and add some more to it later, or you could incorporate mine, or ... something else. Honestly: I'm more interested in having a test co-conspirator at this point, so I'd be happy for you to pick the more funner thing at this moment!

@chrislongros

chrislongros commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @robn. We could first push my own more basic version and then your more extensive version that covers most of the API.

@behlendorf behlendorf left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A trivial conflict to resolve after merging #18638 then this should be all set.

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Jun 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Status: Accepted Ready to integrate (reviewed, tested)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants