Skip to content

mdbx: expose Env.Copy* with MDBX_CP_OVERWRITE and add Env.Defrag - #223

Open
JkLondon wants to merge 13 commits into
masterfrom
jklondon/defrag-cp
Open

mdbx: expose Env.Copy* with MDBX_CP_OVERWRITE and add Env.Defrag#223
JkLondon wants to merge 13 commits into
masterfrom
jklondon/defrag-cp

Conversation

@JkLondon

Copy link
Copy Markdown
Member

Summary

  • Enables the long-commented Env.Copy / CopyFlag / CopyFD / CopyFDFlag methods and rewires them onto the modern mdbx_env_copy(env, dest, flags) / mdbx_env_copy2fd(env, fd, flags) entry points (the old mdbx_env_copy2 / mdbx_env_copyfd2 API the bindings used no longer exists).
  • Adds the missing copy flags as Go constants: CopyDefaults, CopyForceDynamicSize, CopyDontFlush, CopyThrottleMVCC, and CopyOverwrite (= MDBX_CP_OVERWRITE) so callers can clobber an existing target file.
  • Binds mdbx_env_defrag for explicit in-place defragmentation (no full copy needed): introduces Env.Defrag(DefragOptions) (*DefragResult, error), a DefragResult struct mirroring MDBX_defrag_result_t, and constants for MDBX_defrag_stopping_reasons_t (DefragStepSize, DefragLargeChunk, DefragLaggardReader, DefragEnoughThreshold, DefragTimeLimit, DefragAborted, DefragError, ...). The libmdbx-side progress callback is intentionally left out for now (always NULL); the final metrics are still returned.
  • Replaces the long-disabled TestEnv_Copy* skeletons with working tests and adds dedicated coverage for CopyOverwrite and Env.Defrag.

Windows portability

mdbx_filehandle_t is HANDLE (i.e. void*) on Windows and int on POSIX, so writing C.mdbx_filehandle_t(fd) over a Go uintptr does not compile under GOOS=windows. The fix is a tiny cgo wrapper mdbxgo_env_copy2fd(MDBX_env*, uintptr_t, MDBX_copy_flags_t) that performs the platform-specific cast in C, so the Go side stays identical on every target. Cross-checked with GOOS=windows GOARCH=amd64 CGO_ENABLED=1 CC=x86_64-w64-mingw32-gcc go build and go test -c.

Test plan

  • go build ./...
  • go test -count=1 ./mdbx/... (darwin/arm64) — all green, including the new TestEnv_Copy, TestEnv_CopyFD, TestEnv_CopyFlag_Overwrite, TestEnv_Defrag
  • GOOS=windows GOARCH=amd64 CGO_ENABLED=1 CC=x86_64-w64-mingw32-gcc go build ./mdbx/... — cross-compiles cleanly
  • go test -c for the same Windows target — produces a valid PE32+ test binary

JkLondon added 2 commits May 26, 2026 01:32
- Uncomment Env.Copy/CopyFlag/CopyFD/CopyFDFlag and rewire them through
  the unified mdbx_env_copy / mdbx_env_copy2fd entry points.
- Expose MDBX_CP_* flags (CopyDefaults, CopyForceDynamicSize,
  CopyDontFlush, CopyThrottleMVCC and the new CopyOverwrite for
  clobbering an existing target file).
- Bind mdbx_env_defrag via a thin cgo helper (mdbxgo_env_defrag), expose
  DefragOptions / DefragResult and the MDBX_defrag_* stopping reasons.
- Replace the long-commented Copy tests with working ones plus dedicated
  tests for the new CopyOverwrite flag and Env.Defrag.
On Windows mdbx_filehandle_t is HANDLE (void*), so the previous
C.mdbx_filehandle_t(fd) where fd is uintptr would not compile under
GOOS=windows. Route the call through a thin C helper that takes a
uintptr_t and performs the platform-specific cast in C, so the Go side
stays identical on every target. The CopyFD test now runs on Windows
as well.
@JkLondon JkLondon self-assigned this May 25, 2026
JkLondon added 5 commits May 26, 2026 01:42
r.spent_time_dot16 is already typed C.uint64_t, so the explicit
conversion is flagged by unconvert.
mdbx_env_defrag opens its own write transaction via txn_basal_start, and
on Windows it trips ERROR_LOCK_VIOLATION against the env's own LockFileEx
region when run on a handle that has just committed writes. The defrag
API binding itself is fine — only the test scenario hits the libmdbx
Windows locking interaction.
Keep the failure visible in CI until libmdbx clarifies the expected
mdbx_env_defrag calling sequence on Windows / fixes the LockFileEx
region conflict that ERROR_LOCK_VIOLATIONs on a freshly written env.
@AskAlexSharov
AskAlexSharov requested a review from Copilot June 19, 2026 07:32

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copy: a non-compacting copy silently produces an EMPTY database. libmdbx's
copy_asis (v0.14.2) builds the destination meta-pages from a pristine model —
trees.main.root = P_INVALID, first_unallocated = NUM_METAS — and never writes
the source txn's roots or geometry into them, unlike copy_with_compacting.
The data pages are copied byte-for-byte, so the target opens without complaint
and reads as an empty db. That is what the "does not reproduce the committed
data" note in the tests was describing. Reject copies without CopyCompact
(ErrCopyNotCompacting) instead of exposing a backup API that loses data, and
let Copy/CopyFD compact on their own; TestEnv_Copy and TestEnv_CopyFD now
cover the flagless entry points that had no test.

Defrag: open the environment Exclusive in the test, like libmdbx's mdbx_defrag
tool does (MDBX_ENV_DEFAULTS|MDBX_EXCLUSIVE with an MDBX_ACCEDE fallback).
Cutting off trailing pages needs the whole-file lock; taking it per
transaction is what failed the win-2025 job with ERROR_LOCK_VIOLATION. The
requirement is now documented on Env.Defrag.

Lint (7 golangci-lint findings): intrange, fmtappendf, thelper, and the
"shrinked" misspelling — DefragResult.PagesShrinked is now PagesShrunk, with
the wrapper's C field renamed to match.
@AskAlexSharov

Copy link
Copy Markdown
Collaborator

Review pass on 210e400 — three things, one of them a data-loss footgun.

1. Non-compacting copies produce an empty database (fixed by refusing them).

The // a non-compact (CopyDefaults) copy of a freshly-written env does not reproduce the committed data note in the tests is not a test artifact — it is libmdbx losing the data. In copy_asis (mdbx.c, v0.14.2) the destination meta-pages come from meta_init_triplet()meta_model(), which sets trees.gc.root = P_INVALID, trees.main.root = P_INVALID and geometry.first_unallocated = NUM_METAS. Only txnid and the canary are then written into that model — the source's tree roots and geometry never are. copy_with_compacting does write them (meta->trees.main = txn->dbs[MAIN_DBI], meta->geometry.first_unallocated = ...), which is why only the compacting path works.

The data pages are copied faithfully, so the result opens without error and reads as an empty db. Verified on darwin/arm64 across Copy, CopyFD, CopyFlag(CopyDefaults), CopyFDFlag(CopyDefaults), with and without a preceding Sync, and with CopyForceDynamicSize — all four report "key not found"; the same env copied with CopyCompact round-trips fine.

Shipping that as a backup API is worse than not shipping it, so a copy requested without CopyCompact now returns ErrCopyNotCompacting before reaching libmdbx, and Copy/CopyFD compact on their own. That also makes the two flagless entry points testable — TestEnv_Copy and TestEnv_CopyFD now exist and pass. Worth an upstream issue against libmdbx; when it is fixed the guard is one if to delete.

2. win (windows-2025): TestEnv_Defrag failed with ERROR_LOCK_VIOLATION.

Cutting off the trailing pages needs the whole-file lock, and taking it per transaction is what Windows rejected. libmdbx's own mdbx_defrag tool opens with MDBX_ENV_DEFAULTS|MDBX_EXCLUSIVE (falling back to MDBX_ACCEDE on a busy db) for exactly this reason, so the test now opens Exclusive and Env.Defrag documents the requirement. I could not reproduce the Windows path locally — the reasoning is from the libmdbx sources and the tool's own defaults, so this one is on CI to confirm.

3. Lint (7 findings). intrange, fmtappendf, thelper, and the shrinked misspell — DefragResult.PagesShrinked is now PagesShrunk (the wrapper's C field renamed to match; libmdbx's own res.pages_shrinked is untouched). golangci-lint run with the CI's v2.12.2: 0 issues.

Full suite green on darwin/arm64.

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