Skip to content

Check connections when get_client_slow fails #368

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Check connections when get_client_slow fails #368

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
14 changes: 13 additions & 1 deletion sdk/couchbase-core/src/kvclientpool.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,19 @@ where
}
}

self.get_client_slow().await
match self.get_client_slow().await {
Ok(c) => Ok(c),
Err(e) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will end up attempting to rebuild connections when get_client_slow returns Shutdown. I think that the correct approach here might be to not check for a cached error at all in get_client_slow, and to call

self.check_connections().await;
self.rebuild_fast_map().await;

either at that point or do something similar when client creation fails within start_new_client_thread so that we proactively attempt to have a new client ready. What do you think?

As an aside between our developer preview and GA release I'm hoping to rewrite the kvclient, kvclientpool, and kvclientmanager stack as it's too complicated and difficult to understand (especially failure cases)

Copy link
Author

Choose a reason for hiding this comment

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

Fair point. I'll look into the two approaches and update the PR.

// If we failed to get a client, we should check connections and rebuild the fast map.
// For example, when all clients are terminated by server side sending TCP RST,
// the clients will be evicted from the pool, and therefore, when we try to get a client
// from the pool again, the get will fail, and we need to check connections to create new clients.
debug!("Error getting client from pool: {}", e);
self.check_connections().await;
self.rebuild_fast_map().await;
Err(e)
}
}
}

pub async fn close(&self) -> Result<()> {
Expand Down
Loading