Skip to content

test(client): add unit coverage for client utilities and services#19222

Merged
voonhous merged 2 commits into
apache:masterfrom
yihua:test-hudi-client-utils-coverage
Jul 20, 2026
Merged

test(client): add unit coverage for client utilities and services#19222
voonhous merged 2 commits into
apache:masterfrom
yihua:test-hudi-client-utils-coverage

Conversation

@yihua

@yihua yihua commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Describe the issue this Pull Request addresses

hudi-client-common client utilities and services had thin unit coverage. This PR adds focused JUnit 5 tests for two client classes that are straightforward to exercise deterministically without a cluster: the DFS-based lock provider and the table service manager client. Writing the tests surfaced two production bugs, both fixed here.

Summary and Changelog

Adds two new test suites:

  • TestFileSystemBasedLockProvider (org.apache.hudi.client.transaction.lock): exercises FileSystemBasedLockProvider against a local temp directory. Covers acquire/release, contention between two providers over the same lock file (including reading the owner's lock info), expiry-based reclamation (the lock file's modification time is aged deterministically via HoodieStorage.setModificationTime, no sleeps), the zero-expiry case that never reclaims, idempotent unlock, non-reentrancy of the same provider, the default lock path derived from the table base path, negative-expiry validation, and the getLockConfig helper producing usable properties. This consolidates and replaces TestFileBasedLockProvider, which lived in hudi-spark-client despite using no Spark; its unique cases were moved here and the old file is deleted.
  • TestHoodieTableServiceManagerClient (org.apache.hudi.client): mocks the HTTP transport with an in-process com.sun.net.httpserver.HttpServer. Asserts the request path per action (compact / clean / cluster) and the query parameters sent (action, db_name, table_name, basepath, instant, execution_engine, parallelism), plus retry/error handling: a 500 response is retried and surfaces HoodieRemoteException, and an unreachable endpoint also surfaces HoodieRemoteException.

Production fixes surfaced by the new tests:

  • HoodieTableServiceManagerConfig.TABLE_SERVICE_MANAGER_DEPLOY_EXTRA_PARAMS had no default value while its getter uses getStringOrDefault, which throws for no-default properties. Since the getter runs on every compact/clean/cluster call, the table service manager client failed with HoodieException unless hoodie.table.service.manager.deploy.extra.params was explicitly set. The property now defaults to the empty string, matching the sibling deploy properties.
  • FileSystemBasedLockProvider.acquireLock never wrote the owner lock info: the dehadoop refactor ([HUDI-8092] Replace FileSystem and related classes to dehadoop hudi-client-common #11805) moved storage.create(lockFile, false) in front of the pre-existing if (!exists) guard, turning the write into dead code, so lock files have been empty and the owner info embedded in LockManager/TimeGeneratorBase lock-conflict messages has been an empty string. The info is now written unconditionally after the atomic create (create with overwrite=false only succeeds for the file's creator), restoring the original behavior from [HUDI-5377] Write call stack information to lock file #7440.

Impact

The table service manager client now works with default configuration instead of throwing on every call, and filesystem lock files again carry owner/call-stack info for debugging lock conflicts. No API changes.

Risk Level

low

Tests use local filesystem temp directories and an in-process HTTP server on an ephemeral port. No cluster, no external services, and no Thread.sleep-based timing; lock expiry is simulated by setting the lock file modification time. The production changes are a config default and restoring a write guarded by an atomic create.

Documentation Update

hoodie.table.service.manager.deploy.extra.params now documents a default of empty string on the generated config page.

Contributor's checklist

  • Read through contributor's guide
  • Enough context is provided in the sections above
  • Adequate tests were added if applicable

…ss review findings

- Fix two compile errors in TestHoodieTableServiceManagerClient: import
  HoodieTableType from common.model, and declare the checked
  UnsupportedEncodingException from URLDecoder.decode via throws
  IOException.
- TABLE_SERVICE_MANAGER_DEPLOY_EXTRA_PARAMS now defaults to the empty
  string: its getter uses getStringOrDefault, which throws for no-default
  properties, so every compact/clean/cluster call through
  HoodieTableServiceManagerClient failed with HoodieException unless
  hoodie.table.service.manager.deploy.extra.params was explicitly set.
- FileSystemBasedLockProvider.acquireLock writes the owner lock info
  unconditionally after the atomic create: the previous
  'if (!storage.exists(lockFile))' guard ran right after create() and was
  always false, so lock files were empty and lock-conflict messages from
  LockManager and TimeGeneratorBase carried no owner info.
- Consolidate filesystem lock provider tests in hudi-client-common: move
  the default-lock-path and non-reentrancy cases from hudi-spark-client's
  TestFileBasedLockProvider (which used no Spark) into
  TestFileSystemBasedLockProvider and delete the old file.
- Test nits: drop an inert retry-wait property, import TypedProperties
  instead of inline qualification, use assertEquals over
  assertTrue(equals).
@voonhous
voonhous marked this pull request as ready for review July 20, 2026 14:00
@hudi-bot

Copy link
Copy Markdown
Collaborator

CI report:

Bot commands @hudi-bot supports the following commands:
  • @hudi-bot run azure re-run the last Azure build

@voonhous
voonhous merged commit f558c00 into apache:master Jul 20, 2026
75 of 82 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L PR with lines of changes in (300, 1000]

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants