-
Notifications
You must be signed in to change notification settings - Fork 243
Change session/cluster initialization to happen in the background CSHARP-916 #528
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
base: 4.x
Are you sure you want to change the base?
Conversation
This PR doesn't unify Session and Cluster yet but a lot of these changes were made with the unification in mind which will come in a following PR. I'm opening this PR before fixing the test code in order to allow for an earlier review of the API changes/design @jorgebay |
* Remove IDisposable implementation * Move ControlConnection construction to Metadata * Add Metadata InitializeAsync and ShutdownAsync
- add IMetadata interface and change Metadata to be internal - add *Async methods to IMetadata - change internal references from Metadata to InternalMetadata - add *Snapshot methods to IMetadata
Hi @joao-r-reis |
This was being worked on when we had a plan to release a new major version but those plans have for the most part been abandoned for now so there is no timeline for this PR. When the cluster initialization fails, it is recommended to close it and create the cluster object again (from the builder that you already have). If you are building the cluster object and then registering it as a singleton for example, the initialization will happen when the first session is created (likely through the DI container) and if that fails then there is a fail case where all subsequent session creations will fail from that same cluster object. You should create the session during app startup and only then you register it as a singleton (if it fails you let the app crash and the app launch should be retried by whatever deployment tooling you're using as an example, or you can implement retries of this first session creation during startup), or change the DI configuration so that if the first session creation fails then a new cluster object is created. |
@joao-r-reis Thanks for quick response. Can I ask you quick question though a bit off topic. Sometimes our team experience issues with connection pool. No new connections opened. Any suggestions at what direction should we dig to fix this? |
That's too vague, I'd have to take a look at the driver logs (at least INFO level) |
again thank you for looking into. before pasting logs I need to clarify one important thing. in our setup we use a load balancer in front of cassandra. so dotnet app (some api let's say) goes to load balancer which directs request to cassandra itself (it's an Azure's internal load balancer if this is important). (I think this might be a cause of the issue though not concrete explanation has been found so far). logs from latest to oldest: (I tried to leave only meaningful info).
|
Using a load balancer in front of Cassandra is very odd and can lead to issues because the drivers have load balancing policies that maintain a specific number of connections to each node. How are you telling the driver what the addresses of the load balancers are? The driver gets the addresses from the system.local and system.peers tables but I assume those addresses are from the nodes themselves in your setup unless you're using some kind of customized version of Cassandra |
this is cassandra in kubernetes cluster. load balancer used to expose private id to external service (web app in another cluster). at this point, I can't say for 100% how it should be configured correctly. |
Currently
cluster.Connect()
starts the initialization task of the control connection, then creates the session, then initializes the session.This PR changes this behavior so that
cluster.Connect()
creates the session object and returns it to the user immediately. The session initialization task is started in the background which will start the cluster initialization if it's not initialized yet.If a user attempts to do something with the
session
,cluster
orcluster.Metadata
that requires the initialization to be finished, the method will await/block until the initialization task is complete.I also implemented CSHARP-698 on this PR. It uses the reconnection policy to keep retrying the control connection initialization in the background. **After an initialization attempt fails, all method calls (
session.Execute
, etc.) will throw the original exception until a new attempt is started. When a new attempt is started, the methods will block again until the current attempt finishes.This PR also adds
Session.Connect()
for users that want to wait for the initialization to be finished.Several API changes are required in order for this to be a good experience for the user.
The
Cluster.Metadata
property no longer blocks. Also introduced aIMetadata
interface and aIMetadataSnapshotProvider
that declares someMetadata
methods which do not block.ILoadBalancingPolicy.Initialize
is called by the initialization method but aICluster
instance is passed to that method which means that if the user attempts to accessMetadata
it will deadlock because it requires the initialization task to be finished. This PR changes this method so that it receives aIMetadataSnapshotProvider
instance instead (which is inherited byIMetadata
). It also changes the method to support async (returnsTask
).A lot of methods from
ICluster
andISession
are moved toMetadata
. This simplifies the driver's API because these methods and properties were just wrappers of those inMetadata
so it makes more sense to be on theIMetadata
interface only.Also updated the upgrade guide so it's easier to get the overall picture of these API changes by reading the guide.