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.

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

Jump to bottom

Support for connection load balancing across read-replica cluster nodes #22

Merged
merged 9 commits into from
Sep 14, 2024
Merged

Support for connection load balancing across read-replica cluster nodes #22

merged 9 commits into from
Sep 14, 2024

Conversation

ashetkar
Copy link

@ashetkar ashetkar commented Sep 2, 2024 •
edited
Loading

Why

  • Currently, the smart driver does not distinguish between primary nodes and read-replica nodes while distributing the connections.
  • Such a mechanism is required when client wants to have one app talk to read-replica nodes only while another one communicates with primary cluster
  • The IDEA ticket

What

  • Here is the doc which provides the details about the design.
  • This feature is implemented within the existing LoadBalancer implementations, viz. ClusterAwareLoadBalancer and TopologyAwareLoadBalancer
  • The core change is in the method isHostEligible() where, it now additionally checks if a node is of expected type

Testing

  • Added test cases in the driver-examples repository. See PR
  • Performance w.r.t. connection creation is ~6% slower than the vanilla 42.3.5 PGJDBC driver. Added the performance test app used for this measurement in the driver example.

@ashetkar ashetkar marked this pull request as ready for review September 9, 2024 04:40
@ashetkar ashetkar changed the title [Draft] Support for LB across read-replica cluster nodes in JDBC smart driver Support for LB across read-replica cluster nodes in JDBC smart driver Sep 9, 2024
@ashetkar ashetkar changed the title Support for LB across read-replica cluster nodes in JDBC smart driver Support for connection load balancing across read-replica cluster nodes Sep 10, 2024
kneeraj
kneeraj reviewed Sep 10, 2024
View reviewed changes
Copy link
Collaborator

@kneeraj kneeraj left a comment

Choose a reason for hiding this comment

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

Minor comments. Soft approval is there. But will look once more.

README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
pgjdbc/src/main/java/com/yugabyte/ysql/LoadBalanceService.java Outdated Show resolved Hide resolved
kneeraj
kneeraj approved these changes Sep 13, 2024
View reviewed changes
Copy link
Collaborator

@kneeraj kneeraj left a comment

Choose a reason for hiding this comment

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

LGTM. Please take care of the comments.

pgjdbc/src/main/java/com/yugabyte/ysql/ClusterAwareLoadBalancer.java Outdated Show resolved Hide resolved
pgjdbc/src/main/java/com/yugabyte/ysql/ClusterAwareLoadBalancer.java Show resolved Hide resolved
pgjdbc/src/main/java/com/yugabyte/ysql/ClusterAwareLoadBalancer.java Show resolved Hide resolved
pgjdbc/src/main/java/com/yugabyte/ysql/TopologyAwareLoadBalancer.java Show resolved Hide resolved
pgjdbc/src/main/java/com/yugabyte/ysql/TopologyAwareLoadBalancer.java Outdated Show resolved Hide resolved
@ashetkar ashetkar merged commit 5a1f2f0 into master Sep 14, 2024
2 checks passed
@komamitsu komamitsu mentioned this pull request Sep 24, 2024
Merged
ashetkar added a commit that referenced this pull request Oct 7, 2024
@ashetkar
(git cherry-pick -n 5a1f2f0) Support for connection load balancing ac…
2e6a7b4 
…ross read-replica cluster nodes (#22)

* Allow new values of load-balance: any, only-primary, only-rr, prefer-primary and prefer-rr
* Make load-balance property editable in YBClusterAwareDataSource
* Add missing lb properties to PGProperty
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kneeraj kneeraj kneeraj approved these changes

@Sfurti-yb Sfurti-yb Awaiting requested review from Sfurti-yb

@HarshDaryani896 HarshDaryani896 Awaiting requested review from HarshDaryani896

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ashetkar @kneeraj