[Tests] Harden BlobGranuleTest cleanup and correct the blob-granule cleanup docs

Open
  • Is something wrong?
#174 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
57/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
php, postgresql
Domain
databases, documentation, testing

Research direction

Start with tests/Integration/BlobGranuleTest.php and tests/Integration/DatabaseCleanupTrait.php; trace the setup and teardown hooks, metadata clear, and tracked cleanup described in the issue. Then read docs/blob-granules.md and CHANGELOG.md for the listed wording corrections. Done means cleanup recovery and failures are visible, normal data cleanup runs, the dedicated-cluster assumption is documented, and the stated integration checks pass.

Written by the indexing model from the issue text.

Description

good first issue priority:low type:tests

Description

#160 / PR #170 added tearDown() cleanup to tests/Integration/BlobGranuleTest.php and fixed the immediate stack degradation. The cleanup still has these robustness/documentation gaps on master @ 4897d23:

  • Cluster-wide clear. clearBlobRangeMetadata() (:305-315) clears the whole \xff\x02/blobRange/..\xff\x02/blobRange0 system range, not only the ranges this class created (:289-295). That is safe on the dedicated compose cluster (only this class creates ranges, the suite is sequential), but it would delete another actor's metadata on a shared or parallel cluster. It also runs in tearDown() when setUp() skips via markTestSkipped(), so it can clear pre-existing metadata on a non-dedicated cluster.
  • Swallowed failures. The tearDown() loop (:41-47) and clearBlobRangeMetadata() (:312-314) catch every Throwable and only comment. A failed cleanup leaves the suite green and the stack throttled; nothing asserts that \xff\x02/blobRange/ is empty.
  • No recovery in setUp(). Cleanup only runs in tearDown(). An interrupted run (Ctrl-C, killed PHPUnit) can leave keys behind; the next run starts with a throttled cluster and can hang before its first tearDown().
  • Shadowed trait hook. The class's setUp() (:29-34) overrides DatabaseCleanupTrait::setUp() (tests/Integration/DatabaseCleanupTrait.php:28-34), so the trait's "clear all data before each test" does not run. This pre-dates #170, but it leaves the first test and interrupted runs without the intended isolation.
  • Duplicate unblobbify. The first test calls unblobbifyRange() explicitly at :76 and the tracked cleanup registered at :64 calls it again in tearDown(). Harmless, but an unnecessary system-key write.
  • Docs/CHANGELOG wording. docs/blob-granules.md:189-195 says ratekeeper "treats any key there as an active range"; the value stays inactive, but checkAnyBlobRanges() counts any key. It does not state the dedicated-cluster assumption of the broad clear. CHANGELOG.md:10 says the test "clears the blob-range metadata it creates", but it clears the whole range. docs/blob-granules.md:14,105 says "blob-manager workers" and :174 says a read "needs a real loader"; FDB has a blob manager and separate blob workers, and the skip is due to missing workers/granule files, not a missing loader implementation.

Impact: a failed or interrupted cleanup can silently leave the stack throttled to zero TPS, and the docs overstate the scope of the cleanup. The completed-run path is correct.

Where to start

  • Call clearBlobRangeMetadata() at the start of setUp() (after getDatabase(), before enableBlobGranules()) so an interrupted run can recover; keep the tearDown() call.
  • Log cleanup failures to STDERR and, after the clear, read \xff\x02/blobRange/..\xff\x02/blobRange0 and fail if any key remains.
  • Document the dedicated-cluster assumption in docs/blob-granules.md (or narrow the clear to tracked KRM boundaries; that is more complex because the KRM coalesces boundaries).
  • Alias the trait's setUp() like tearDown and call it, or call $this->getDatabase()->clearAll() explicitly in the class's setUp().
  • Remove one of the duplicate unblobbify calls in the first test, or keep it as a safety net with a comment.
  • Fix the docs/CHANGELOG wording listed above.

Definition of done

  • An interrupted run leaves no keys in \xff\x02/blobRange/, or the next setUp() clears them before the first test.
  • A cleanup failure is visible (logged or failed assertion), not silently swallowed.
  • The class clears normal data before the first test (trait setUp() is called or clearAll() is explicit).
  • The dedicated-cluster assumption and the meaning of the metadata clear are documented; blob-manager workers/real loader and the CHANGELOG wording are corrected.
  • make test-integration is green and the suite leaves raw \xff\x02/blobRange/ keys at 0.
Dominant language
PHP
Stars
0
Forks
0
Avg merge
8h 20m
Merged PRs (30d)
23

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from crazy-goat/fdb-php

All issues in crazy-goat/fdb-php

Similar issues

More PHP issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.