Skip to content

refactor: start a meta-service as local meta for testing #17821

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

drmingdrmer
Copy link
Member

@drmingdrmer drmingdrmer commented Apr 21, 2025

I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/

Summary

refactor: start a meta-service as local meta for testing
  • Replaced the in-memory meta-store with a meta-service running in a
    temporary directory for testing purposes.

  • The in-memory meta-store was limited in functionality and did not
    provide a complete feature set.

  • The new approach ensures full functionality during testing by
    simulating a real meta-service.

Tests

  • Unit Test
  • Logic Test
  • Benchmark Test
  • No Test - Explain why

Type of change

  • Refactoring

Related Issues


This change is Reviewable

@github-actions github-actions bot added the pr-refactor this PR changes the code base without new features or bugfix label Apr 21, 2025
@drmingdrmer drmingdrmer force-pushed the 305-local-meta branch 3 times, most recently from a7d2575 to 4d7583b Compare April 23, 2025 06:04
@drmingdrmer drmingdrmer force-pushed the 305-local-meta branch 4 times, most recently from 11fcf47 to 0e27dc0 Compare May 6, 2025 12:12
- Replaced the in-memory meta-store with a meta-service running in a
  temporary directory for testing purposes.

- The in-memory meta-store was limited in functionality and did not
  provide a complete feature set.

- The new approach ensures full functionality during testing by
  simulating a real meta-service.
Fix a timing issue in the time-based semaphore test where closely created
semaphores could lead to incorrect behavior. Ensures that distinguished
semaphores have sufficiently large timestamp gaps between them.

The time-based semaphore mechanism requires clearly separated timestamps
to function properly, as a lower timestamp semaphore inserted after a
higher timestamp one could result in both being incorrectly acquired.
@drmingdrmer drmingdrmer requested review from zhang2014 and Copilot May 6, 2025 15:24
@drmingdrmer drmingdrmer marked this pull request as ready for review May 6, 2025 15:24
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the meta-store for testing by replacing the in-memory implementation with a local meta-service, simulating real meta-service behavior. Key changes include adding delayed sleeps in tests to account for timestamp discrepancies, updating async work to use the new meta-service API, and refactoring dependency usage in both testing and production modules.

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/query/service/tests/it/sessions/queue_mgr.rs Added timed delays to ensure correct semaphore ordering in tests
src/query/service/src/sessions/queue_mgr.rs Added Debug derivation for AcquireQueueGuard
src/query/management/tests/it/workload.rs Converted workload manager creation to async with updated meta-store API
src/query/management/tests/it/warehouse.rs Updated warehouse tests to construct local meta-service from the new API
src/meta/store/* Replaced MemMeta with LocalMetaService and updated tests accordingly
src/meta/api/src/txn_backoff.rs Adjusted backoff test tolerance to account for increased timing variation
Comments suppressed due to low confidence (1)

src/meta/api/src/txn_backoff.rs:143

  • [nitpick] Update the assertion message to better reflect the expected timing behavior now that the tolerance has increased from 0.010 to 0.100 seconds, minimizing potential confusion during test failures.
assert!(elapsed < 0.100, "{} is expected to be 0", elapsed);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-refactor this PR changes the code base without new features or bugfix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant