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
Cassandra Clustering implementation #8925
base: main
Are you sure you want to change the base?
Conversation
29937b9
to
884e6b4
Compare
@dotnet-policy-service agree company="Microsoft" |
@ReubenBond Can this PR get flagged to run the integration tests? Specifically to validate that the new Cassandra tests:
|
@rkargMsft done |
4ff58e7
to
84d0649
Compare
@ReubenBond Sorry, looks like this needs approval for each commit to run the tests again |
84d0649
to
145d828
Compare
@ReubenBond orleans/test/Extensions/Tester.Cassandra/Clustering/Cassandra.cs Lines 103 to 157 in 84d0649
This test expects to reject the insert of a membership entry if it already exists for the specific Silo. Cassandra doesn't have a way to support this: doing both a Light Weight Transaction (LWT) for the table version check and an Exists check for the entry table isn't something that it supports. That said, it seems like it would require an incorrectly functioning Silo to even get in this situation. It would need to read the current membership table, see that an entry exists for this Silo, decide to insert a new entry anyway, increment to the correct New Table version, then insert the entry. No other Silo would be adding an entry that would conflict, so I think this would be something that wouldn't ever happen. |
@rwkarg that is very odd. Are you saying that |
Can't do both an |
@ReubenBond Found a better way to explain the situation. Where this comes up is that the membership table has rows for each Silo entry but there's a
Cassandra doesn't directly support doing both of those in a query. The recommendations I've seen to work around this are to have the application logic do the "does this row exist" check based on querying the data and then make the write as an MembershipTableManager already makes this application side check by:
|
Given we have a version check, can the Cassandra implementation fail the insert if it knows that the version row is there already? The version check should be all we need here, given we have lightweight transactions for consistency
Essentially, this. If MembershipTableManager is already performing the check, then the behavior checked by this test shouldn't be necessary. |
Yes, this Cassandra implementation does that version check and will fail the query if the version is mismatched (this is covered by other tests). That was the only open question I had. Otherwise I believe this is good to go, aside from the two Azure Pipeline check failures. |
145d828
to
530b2b7
Compare
@ReubenBond Are the failing dotnet.orleans Azure Pipelines checks something that I need to address? It looks like it's running on Windows and there aren't any Cassandra container images for Windows. |
This appears to be required for some of the test projects to run, even though no Cassandra tests end up using it.
1c8bd55
to
1535cab
Compare
Built off of https://github.com/OrleansContrib/OrleansCassandraUtils with the following major changes:
Microsoft Reviewers: Open in CodeFlow