Skip to content
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

Support Alibaba pseudo-cluster configurations #2646

Merged
merged 3 commits into from
Feb 13, 2024
Merged

Support Alibaba pseudo-cluster configurations #2646

merged 3 commits into from
Feb 13, 2024

Conversation

mgravell
Copy link
Collaborator

@mgravell mgravell commented Feb 11, 2024

1: don't treat trivial clusters as clusters - Alibaba uses this config
2: report synchronous failures immidiately and accurately

fix #2642

For context, Alibaba reports from cluster nodes as 2 nodes:

aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa {proxy endpoint} myself,master - 0 0 0 connected 0-16383
bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb {proxy endpoint} slave aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa 0 0 0 connected

which confuses things somewhat; this isn't a valid/normal cluster configuration, so: ignore it!

Other relevant metadata:

from config:

databases=256

from info:

# Cluster
cluster_enabled:1
databases:256
nodecount:2

1: don't treat trivial clusters as clusters - Alibaba uses this config
2: report synchronous failures immidiately and accurately
@mgravell mgravell requested a review from NickCraver February 11, 2024 15:43
@NickCraver
Copy link
Collaborator

How does this affect single node clusters? For example, even in Azure hosting people start with a single cluster node in a lot of cases which makes scaling to n easier later...but they don't support SELECT because they are cluster. I worry about the assumption of 2+ here being needed.

@mgravell
Copy link
Collaborator Author

Untested and it is a good question, but: if the worst side effect here is that we get a server-side "nope" rather than a client-side "nope" in genuine single-server clusters: I'm comfortable with that...?

@NickCraver
Copy link
Collaborator

I used SELECT as an example, but I think this had many more side-effects such as not detecting when the cluster changes and such. If for example we were viewing this in Opserver…it'd be wrong. Any code basing off cluster or not on top of us has the same downstream fun. Lots of potential there.

I'd vote we should not make such an assumption based on node count - likely better for the server-side to not represent itself as a cluster here.

@mgravell
Copy link
Collaborator Author

@NickCraver thanks for keeping me honest; refactored to use the "databases" entry as reported from config; this data is also available under info cluster, but I don't propose to add an extra info call unless this doesn't work reliably

Copy link
Collaborator

@NickCraver NickCraver left a comment

Choose a reason for hiding this comment

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

ADD SOME RELEASE NOTES YOU HEATHEN

@mgravell mgravell merged commit 39eac01 into main Feb 13, 2024
4 checks passed
@mgravell mgravell deleted the alibaba branch February 13, 2024 16:07
@yangbodong22011
Copy link

@mgravell Thank you for your quick fix. We have verified the 2.7.20 version of MyGet and it works very well. When do you expect to release it to NuGet? After that, we will push users to upgrade.

@mgravell
Copy link
Collaborator Author

Thanks for confirming. I'll discuss release in our weekly sync tomorrow (20th) - assuming there's no known blockers, I expect we'll release to NuGet either end-of-day 20th, or very shortly after

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.

Feature Request: DB selection support for proxy-bridged cluster
3 participants