Skip to content

Checkpoint(ClientInfo) Caching System #1424

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.

Sign up for GitHub

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

Closed
mnnseong wants to merge 14 commits into from
Closed

Checkpoint(ClientInfo) Caching System #1424

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
5903d4c
Checkpoint(ClientInfo) Caching System
mnnseong Aug 1, 2025
b0fd6c8
concurrency bug fix
mnnseong Aug 4, 2025
b5c2e87
resolve linter errors
mnnseong Aug 4, 2025
06ecf54
Refactor cache structure and add state caching
mnnseong Aug 7, 2025
a6a79e4
Refactor metadata upsert logic and enable cleanup
mnnseong Aug 11, 2025
d8b6aa5
write-through client state with CAS
mnnseong Aug 18, 2025
93a65db
Merge branch 'main' into cp-caching
mnnseong Aug 18, 2025
5cce717
Revise code with coderabbit
mnnseong Aug 20, 2025
8002813
Implement TTL & Routing Test
mnnseong Aug 21, 2025
a3dc337
Resolve linter error
mnnseong Aug 21, 2025
3d5046e
adjust TTL and flush interval
mnnseong Aug 21, 2025
bd40e7b
Resolve data race
mnnseong Aug 21, 2025
6e420fd
trigger CI
mnnseong Aug 21, 2025
0482f5f
Revise Code
mnnseong Aug 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions server/backend/backend.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,12 @@ func (b *Backend) Shutdown() error {
b.EventWebhookManager.Close()
b.ClusterClient.Close()

if mongoClient, ok := b.DB.(*mongo.Client); ok {
if err := mongoClient.CloseClientInfoCache(); err != nil {
logging.DefaultLogger().Error("close client info cache: " + err.Error())
}
}

if err := b.MsgBroker.Close(); err != nil {
logging.DefaultLogger().Error(err)
}
Expand Down
4 changes: 4 additions & 0 deletions server/backend/database/database.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,10 @@ type Database interface {
// DeactivateClient deactivates the client of the given refKey.
DeactivateClient(ctx context.Context, refKey types.ClientRefKey) (*ClientInfo, error)

// DeactivateClientForHousekeeping deactivates the client for housekeeping purposes.
// This method bypasses cache and directly updates the database.
DeactivateClientForHousekeeping(ctx context.Context, refKey types.ClientRefKey) (*ClientInfo, error)

Comment on lines +189 to +192

@coderabbitai coderabbitai Bot Aug 18, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Clarify contract and align semantics across backends for housekeeping deactivation

Right now, Mongo returns (nil, nil) when the client is not found or already deactivated, while the memory backend can return a non-nil ClientInfo in the "already deactivated" case. Please document the intended contract here and align implementations accordingly.

I recommend standardizing on: return (nil, nil) if the client does not exist, belongs to another project, or is already deactivated. This enables callers (e.g., housekeeping) to treat nil as "no-op" consistently.

Apply this comment update to make the contract explicit:

-	// DeactivateClientForHousekeeping deactivates the client for housekeeping purposes.
-	// This method bypasses cache and directly updates the database.
+	// DeactivateClientForHousekeeping deactivates the client for housekeeping purposes.
+	// This method bypasses any in-memory cache and directly updates the database.
+	// Returns (nil, nil) if the client does not exist, is not in the given project,
+	// or is already deactivated (i.e., no state change was performed).
 	DeactivateClientForHousekeeping(ctx context.Context, refKey types.ClientRefKey) (*ClientInfo, error)

🏁 Script executed:

#!/bin/bash
set -e
echo "Searching for DeactivateClientForHousekeeping occurrences in Go files"
rg -n "DeactivateClientForHousekeeping" -C2 --type=go

Length of output: 2279

🏁 Script executed:

#!/bin/bash
set -e
echo "Extracting memory DB implementation for DeactivateClientForHousekeeping:"
sed -n '860,950p' server/backend/database/memory/database.go
echo
echo "Extracting Mongo client implementation for DeactivateClientForHousekeeping:"
sed -n '1000,1100p' server/backend/database/mongo/client.go

Length of output: 6175

Clarify DeactivateClientForHousekeeping contract and align semantics across backends

The memory backend still returns a non-nil ClientInfo when the client is already deactivated, whereas the Mongo backend returns (nil, nil) in that case. Let’s standardize on returning (nil, nil) whenever no state change is needed (client not found, wrong project, or already deactivated).

• server/backend/database/database.go (interface): update doc comment
• server/backend/database/memory/database.go: change the “already deactivated” branch to return nil, nil

Suggested doc-comment diff in database.go:

-	// DeactivateClientForHousekeeping deactivates the client for housekeeping purposes.
-	// This method bypasses cache and directly updates the database.
+	// DeactivateClientForHousekeeping deactivates the client for housekeeping purposes.
+	// This method bypasses any in-memory cache and directly updates the database.
+	// Returns (nil, nil) if the client does not exist, is not in the given project,
+	// or is already deactivated (i.e., no state change was performed).

Then, in memory/database.go, replace:

if clientInfo.Status != database.ClientActivated {
    return clientInfo, nil // Already deactivated, return current state
}

with:

if clientInfo.Status != database.ClientActivated {
    return nil, nil // Already deactivated, no-op
}
🤖 Prompt for AI Agents 
In server/backend/database/database.go around lines 189-192, update the
DeactivateClientForHousekeeping doc comment to state that the method returns
(nil, nil) when no state change is needed (client not found, wrong project, or
already deactivated); then in server/backend/database/memory/database.go locate
the branch that checks if clientInfo.Status != database.ClientActivated and
change its return to nil, nil (instead of returning the current ClientInfo) so
the memory backend semantics match Mongo and the interface doc.

// TryAttaching updates the status of the document to Attaching to prevent
// deactivating the client while the document is being attached.
TryAttaching(ctx context.Context, refKey types.ClientRefKey, docID types.ID) (*ClientInfo, error)
Expand Down
49 changes: 49 additions & 0 deletions server/backend/database/memory/database.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -912,6 +912,55 @@ func (d *DB) DeactivateClient(_ context.Context, refKey types.ClientRefKey) (*da
return clientInfo, nil
}

// DeactivateClientForHousekeeping deactivates the client for housekeeping purposes.
// This method bypasses any in-memory cache and directly updates the database.
// Returns (nil, nil) if the client does not exist, is not in the given project,
// or is already deactivated (i.e., no state change was performed).
func (d *DB) DeactivateClientForHousekeeping(
_ context.Context,
refKey types.ClientRefKey,
) (*database.ClientInfo, error) {
if err := refKey.ClientID.Validate(); err != nil {
return nil, err
}

txn := d.db.Txn(true)
defer txn.Abort()

raw, err := txn.First(tblClients, "id", refKey.ClientID.String())
if err != nil {
return nil, fmt.Errorf("find client by id: %w", err)
}

// Client not found, return nil to indicate no action needed
if raw == nil {
return nil, nil
}

// Client not in project, return nil to indicate no action needed
clientInfo := raw.(*database.ClientInfo).DeepCopy()
if err := clientInfo.CheckIfInProject(refKey.ProjectID); err != nil {
return nil, nil
}

// Only deactivate if currently activated
if clientInfo.Status != database.ClientActivated {
return nil, nil
}

// clientInfo.Status = database.ClientDeactivated
// clientInfo.UpdatedAt = gotime.Now()

clientInfo.Deactivate()

if err := txn.Insert(tblClients, clientInfo); err != nil {
return nil, fmt.Errorf("update client: %w", err)
}
txn.Commit()

return clientInfo, nil
}

// FindClientInfoByRefKey finds a client by the given refKey.
func (d *DB) FindClientInfoByRefKey(_ context.Context, refKey types.ClientRefKey) (*database.ClientInfo, error) {
if err := refKey.ClientID.Validate(); err != nil {
Expand Down
Loading
Loading