Skip to content

refactor: Hold service pointer in database instance#763

Merged
zhanglei1949 merged 7 commits into
alibaba:mainfrom
zhanglei1949:zl/fix-db-svc-rel
Jul 23, 2026
Merged

refactor: Hold service pointer in database instance#763
zhanglei1949 merged 7 commits into
alibaba:mainfrom
zhanglei1949:zl/fix-db-svc-rel

Conversation

@zhanglei1949

Copy link
Copy Markdown
Member

Fix #757

@zhanglei1949 zhanglei1949 changed the title let db hold pointer to service fix: Hold service pointer in database instance Jul 20, 2026

Copilot AI 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.

Pull request overview

This PR addresses Issue #757 by enforcing a single active NeugDBService per NeugDB instance, tracking service association inside the database and rejecting conflicting operations while serving.

Changes:

  • Add NeugDBNeugDBService association tracking via an atomic active_service_, with register/unregister hooks and a HasActiveService() query.
  • Reject NeugDBService creation when one is already associated with the same NeugDB, and reject NeugDB::Connect() / NeugDB::Close() while a service is active.
  • Add unit tests covering single-service enforcement, init-failure rollback, and blocking connect/close while serving.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/unittest/test_db_svc.cc Adds unit tests for single-service enforcement and blocked DB operations while serving.
src/server/neug_db_service.cc Unregisters the service from the DB on destruction.
src/main/neug_db.cc Implements service registration tracking; blocks Connect()/Close() while serving; adjusts destructor close behavior.
include/neug/server/neug_db_service.h Registers/unregisters service association during construction (with rollback on init failure) and documents the constraint.
include/neug/main/neug_db.h Declares service association API and stores the active service pointer atomically.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/main/neug_db.cc
Comment thread src/main/neug_db.cc Outdated
@zhanglei1949 zhanglei1949 changed the title fix: Hold service pointer in database instance refactor: Hold service pointer in database instance Jul 20, 2026
Copilot AI review requested due to automatic review settings July 23, 2026 08:45

Copilot AI 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.

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Comment thread include/neug/main/neug_db.h Outdated

// The NeugDBService currently associated with this database, nullptr if
// none. At most one service can be associated at any given time.
std::atomic<NeugDBService*> active_service_{nullptr};

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why do we need atomic here,Is there a race condition?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Changed to raw pointer. The thread safety will be guarded by the mutex.

Copilot AI review requested due to automatic review settings July 23, 2026 10:09

Copilot AI 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.

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

@zhanglei1949
zhanglei1949 merged commit d343381 into alibaba:main Jul 23, 2026
14 of 15 checks passed
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.

Multiple NeugDBService instances can attach to one NeugDB, causing concurrent modification of shared db state

3 participants