Feature: enable multi pool client connections #577

sinadarbouy · 2024-07-14T17:19:17Z

Ticket(s)

Description

This pull request introduces significant updates to enhance the handling of proxies within the server configuration and adds support for load balancing strategies. Below are the key changes and features included in this pull request:

Changes

Configuration Changes:

Updated the configuration files (config.go, constants.go, types.go) to include new fields for multiple proxies and load balancer strategies.
Added default values for new fields in config.go.
Enhanced the global configuration validation to ensure proper referencing of clients, pools, and proxies.
Important: The structure of the gatewayd.yaml configuration has changed to reflect the new support for multiple proxies and load balancer strategies. Specifically, the configuration now includes a loadBalancer section where rules and strategies are grouped together for clarity.

Load Balancer Implementation:

Implemented loadbalancer.go to handle load balancing strategy selection.
Introduced round-robin.go to implement the Round Robin load balancing strategy.
Created tests for load balancer strategies (loadbalancer_test.go, round-robin_test.go).

Configuration Examples:

Updated gatewayd.yaml to include examples of multiple proxies and load balancer configuration.

# GatewayD Global Configuration

loggers:
  default:
...

metrics:
  default:
...

clients:
  default:
...
  default-2:
...

pools:
  default:
...
  default-2:
...

proxies:
  default:
...
  default-2:
...

servers:
  default:
    network: tcp
    address: 0.0.0.0:15432
    proxies:
      - "default"
      - "default-2"
    loadBalancer:
      strategy: ROUND_ROBIN
      # Not yet implemented.
      # loadBalancingRules:
      #   - condition: "default"
      #     percentages:
      #       - proxy: "default"
      #         percentage: 70
      #       - proxy: "default-2"
      #         percentage: 30

api:
...

Related PRs

Development Checklist

Legal Checklist

I have read the CONTRIBUTING document.
I have read and understood the Code of Conduct.
I have read and agreed to the Apache CLA (required).
I have read and agreed to the LICENSE (required).

config/config.go

gatewayd.yaml

network/loadbalancer.go

network/server.go

network/round-robin.go

config/config.go

network/loadbalancer_test.go

network/round-robin_test.go

network/server.go

ccoVeille · 2024-07-17T18:39:15Z

LGTM 👍

network/round-robin.go

mostafa

Thanks for your awesome contribution! 🙏 This paves the way for further development.

I have left a few comments.

errors/errors.go

gatewayd.yaml

network/constants.go

network/loadbalancer.go

network/server.go

network/round-robin_test.go

network/round-robin.go

network/round-robin_test.go

This commit introduces significant updates to enhance the handling of proxies within the server configuration and adds support for load balancing strategies. Changes: - Updated API tests to reflect changes from a single `Proxy` to a list of `Proxies`. - Adjusted initialization and configuration of proxies in `run.go` to support multiple proxies and load balancer strategies. - Updated configuration files to include new fields for multiple proxies and load balancer strategies. - Enhanced global configuration validation for clients, pools, and proxies. - Added new `loadBalancer` section in `gatewayd.yaml` for rules and strategies. - Implemented load balancing strategy selection and Round Robin strategy. - Added tests for load balancer strategies. - Added new error type `ErrorCodeLoadBalancerStrategyNotFound`. - Improved proxy connection handling and added informative comments. Configuration Example: - Updated `gatewayd.yaml` to reflect new support for multiple proxies and load balancer strategies. - Ensure to update your configuration files accordingly. Testing: - Updated existing tests and added new tests for multi-proxy and load balancing functionality. - Verified configuration validation for proxies and load balancers. Impact: - Improved flexibility and scalability of server configuration. - Enabled robust proxy management and efficient load distribution. Update errors/errors.go Co-authored-by: Mostafa Moradian <[email protected]> Signed-off-by: sina <[email protected]> fixed review problems

ccoVeille

LGTM 🤞

- Change pools, clients, and proxies to use map[string]map[string] instead of map[string] - Update related code for compatibility with new nested map structure

remvoed server 2 from gatewatd.yaml file

…iles Updated variable names in the API, test files, run command, and configuration files to improve code readability and maintainability. Key changes include: 1. **api.go**: - Renamed `configurationGroupPools` to `configGroupPools` for clarity. 2. **api_helpers_test.go, api_test.go**: - Updated to use `config.DefaultConfigurationBlock` instead of `config.DefaultPool` and `config.DefaultProxy` for better naming consistency. 3. **run.go**: - Changed iteration variables to `configGroup` and `configBlockName` for better descriptive naming. - Updated logging and error handling to use `configBlockName`. - Modified span attribute naming for tracing proxy creation. 4. **config.go**: - Updated `DefaultPool`, `DefaultClient`, and `DefaultProxy` to `DefaultConfigurationBlock` in the `LoadDefaults` function. - Enhanced configuration parsing logic to handle `DefaultConfigurationBlock` appropriately and ensure robust error handling. 5. **constants.go**: - Renamed `DefaultClient`, `DefaultPool`, and `DefaultProxy` to `DefaultConfigurationBlock`. - Retained other configuration constants for environment prefix, tracer name, and configuration filenames. These changes enhance the codebase by standardizing variable names, improving readability, and making the code easier to maintain.

- Enhanced the Client struct by adding YAML tags to all fields, ensuring compatibility with YAML parsers. - Added YAML tags to the Size field in the Pool struct. - Included YAML tags for the HealthCheckPeriod field in the Proxy struct. These changes address limitations of the YAML parser and ensure proper serialization and deserialization of configuration data.

sinadarbouy · 2024-07-28T12:38:11Z

recent changes I made to the structure configuration. We were having issues, especially when dealing with multiple servers. If a second server went down, no logs were being generated. This happened because the logger was expecting a second instance (default-2), and when it wasn’t there, the logger wouldn’t work, and we couldn’t see any logs. Adding a second logger instance made the errors visible. Also, the metrics system was looking for default-2 and throwing errors when it couldn’t find it.

To fix this, I changed the structure to match what was originally suggested in issue #398. Now, each server, client, pool, and proxy can have its own logger and metrics configuration, which should make things more reliable and scalable.

gatewayd.yaml

api/api.go

mostafa · 2024-07-28T18:17:36Z

api/api.go

-		pools[name] = map[string]interface{}{
-			"cap":  p.Cap(),
-			"size": p.Size(),
+	pools := make(map[string]any)


Note to self: plugins should be updated as well, as they rely on this API.

api/api_test.go

cmd/run.go

config/types.go

ccoVeille · 2024-07-28T18:43:35Z

gatewayd.yaml

-    backoff: 1s # duration
-    backoffMultiplier: 2.0 # 0 means no backoff
-    disableBackoffCaps: false
+    activeWrites:


The fact this node appears let me think (I might be wrong) this is a breaking changes.

Is it true? Ia everyone OK with that?

This change is backward incompatible by design. We can have backward compatibility, but it will increase maintenance overhead in the long run. I'll add a warning to the release notes for this.

ccoVeille · 2024-07-28T18:44:46Z

gatewayd.yaml

+      backoff: 1s # duration
+      backoffMultiplier: 2.0 # 0 means no backoff
+      disableBackoffCaps: false
+    standbyReads:


I find no reference of this on the code, while I found activeWrites.

Based on the code, I would say the code will look for any node.

So, is there a need to enforce people to use activeWrites node?

Couldn't we work on a logic of weight or something like that?(maybe another PR) So when no weight each node are equivalent

The activeWrites and standbyReads can be any name. When we created the nested structure for Clients, Pools, and Proxies, we needed to set names for them. I didn’t want to use default because we already have a default for the configuration group, which would be confusing. Therefore, I used activeWrites (copied from the issue description). Now, the default name for the configuration block (nested name for Clients, Pools, Proxies) is activeWrites. That’s why you will find references to activeWrites. standbyReads was added to gateway.yaml to follow the issue structure, meaning that the gateway can support multiple Proxies(maybe its better to remove it).

Co-authored-by: Mostafa Moradian <[email protected]> Signed-off-by: sina <[email protected]>

mostafa · 2024-07-28T19:08:41Z

The GetServers endpoint in the API should return the running loadbalancer config as well:
https://github.com/gatewayd-io/gatewayd/pull/577/files#diff-bf98d5fab5bcfabded9769069e7318683f549ec91cff84c5f87a9ae3a3ca3d8dR280

- Modified the GetServers endpoint to return the current load balancer configuration for each server. - Added a new field "loadBalancer" to the server details, including the load balancer strategy name. - Updated unit tests to validate the new "loadBalancer" field in the API response. - Adjusted test setup to include the default load balancer strategy configuration.

Renamed short variable names used in type assertions to longer, more descriptive names to resolve golint warnings about variable name length.

…strategies.

mostafa

LGTM! 🎉

@sinadarbouy Thanks for all your efforts and contribution! 🙏
@ccoVeille Thanks for the reviews! 🙏
@likecodingloveproblems Thanks for the initial PR! 🙏

I'll merge this PR and more load balancing strategies and features can be added in future PRs. We may also need to create seprate PRs for those features (and fixes).

sinadarbouy force-pushed the feature/enable-multi-pool-client-connections branch from 04334b8 to d35c2ab Compare July 14, 2024 17:26

ccoVeille reviewed Jul 14, 2024

View reviewed changes

ccoVeille reviewed Jul 15, 2024

View reviewed changes

config/config.go Outdated Show resolved Hide resolved

config/config.go Outdated Show resolved Hide resolved

ccoVeille reviewed Jul 16, 2024

View reviewed changes

config/config.go Outdated Show resolved Hide resolved

network/loadbalancer_test.go Outdated Show resolved Hide resolved

network/loadbalancer_test.go Outdated Show resolved Hide resolved

network/round-robin_test.go Outdated Show resolved Hide resolved

network/server.go Show resolved Hide resolved

sinadarbouy force-pushed the feature/enable-multi-pool-client-connections branch from f90c704 to 391ee7d Compare July 17, 2024 20:26

sinadarbouy marked this pull request as ready for review July 17, 2024 20:35

likecodingloveproblems reviewed Jul 18, 2024

View reviewed changes

network/round-robin.go Outdated Show resolved Hide resolved

sinadarbouy mentioned this pull request Jul 22, 2024

Enable multi-pool client connections #398

Closed

6 tasks

mostafa reviewed Jul 23, 2024

View reviewed changes

sinadarbouy force-pushed the feature/enable-multi-pool-client-connections branch 2 times, most recently from 016dac1 to 491cad3 Compare July 23, 2024 18:32

ccoVeille reviewed Jul 23, 2024

View reviewed changes

network/round-robin.go Outdated Show resolved Hide resolved

network/round-robin_test.go Outdated Show resolved Hide resolved

sinadarbouy force-pushed the feature/enable-multi-pool-client-connections branch from 491cad3 to 585ed7f Compare July 23, 2024 20:11

ccoVeille reviewed Jul 23, 2024

View reviewed changes

sinadarbouy added 6 commits July 26, 2024 14:47

Refactor maps to use nested structure for pools, clients, and proxies

1326cc8

- Change pools, clients, and proxies to use map[string]map[string] instead of map[string] - Update related code for compatibility with new nested map structure

applied nested changes to all yaml files

890a2d4

remvoed server 2 from gatewatd.yaml file

added required parameters to test data

754a426

removed extra spaces in gatewayd.yaml

52a229b

ccoVeille suggested changes Jul 28, 2024

View reviewed changes

gatewayd.yaml Outdated Show resolved Hide resolved

gatewayd.yaml Outdated Show resolved Hide resolved

gatewayd.yaml Show resolved Hide resolved

Refactor: Changed kebab-case to camelCase for configuration blocks

9328b80

mostafa reviewed Jul 28, 2024

View reviewed changes

ccoVeille reviewed Jul 28, 2024

View reviewed changes

Avoid using the request context for tracing in the GetPools method.

5bd4f5c

Co-authored-by: Mostafa Moradian <[email protected]> Signed-off-by: sina <[email protected]>

fix: correct typo in configBlockName

415b58e

sinadarbouy added 2 commits July 28, 2024 21:20

removed extra _ for GetPools

c72f59f

interface{} replaced with any

d309592

mostafa changed the title ~~Feature/enable multi pool client connections~~ Feature: enable multi pool client connections Jul 28, 2024

sinadarbouy added 3 commits July 28, 2024 21:55

Fix variable name 'ok' to adhere to golint guidelines

85f7102

Renamed short variable names used in type assertions to longer, more descriptive names to resolve golint warnings about variable name length.

Added comment in gatewayd.yaml to indicate location of load balancer …

905bc10

…strategies.

mostafa approved these changes Jul 28, 2024

View reviewed changes

mostafa merged commit fd147b4 into gatewayd-io:main Jul 28, 2024
5 checks passed

mostafa linked an issue Jul 28, 2024 that may be closed by this pull request

Enable multi-pool client connections #398

Closed

6 tasks

This was referenced Jul 31, 2024

Env-vars expect lower case letters #583

Closed

Update env-var structure gatewayd-io/docs#62

Closed

Fix casting of client connections upon enumeration #584

Merged

mostafa mentioned this pull request Oct 6, 2024

Investigate other proxy mechanisms #425

Open

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: enable multi pool client connections #577

Feature: enable multi pool client connections #577

sinadarbouy commented Jul 14, 2024 •

edited by mostafa

Loading

ccoVeille commented Jul 17, 2024

mostafa left a comment

ccoVeille left a comment

sinadarbouy commented Jul 28, 2024

mostafa Jul 28, 2024

ccoVeille Jul 28, 2024

mostafa Jul 28, 2024 •

edited

Loading

ccoVeille Jul 28, 2024

sinadarbouy Jul 28, 2024

mostafa commented Jul 28, 2024

mostafa left a comment

Feature: enable multi pool client connections #577

Feature: enable multi pool client connections #577

Conversation

sinadarbouy commented Jul 14, 2024 • edited by mostafa Loading

Ticket(s)

Description

Changes

Configuration Changes:

Load Balancer Implementation:

Configuration Examples:

Related PRs

Development Checklist

Legal Checklist

ccoVeille commented Jul 17, 2024

mostafa left a comment

Choose a reason for hiding this comment

ccoVeille left a comment

Choose a reason for hiding this comment

sinadarbouy commented Jul 28, 2024

mostafa Jul 28, 2024

Choose a reason for hiding this comment

ccoVeille Jul 28, 2024

Choose a reason for hiding this comment

mostafa Jul 28, 2024 • edited Loading

Choose a reason for hiding this comment

ccoVeille Jul 28, 2024

Choose a reason for hiding this comment

sinadarbouy Jul 28, 2024

Choose a reason for hiding this comment

mostafa commented Jul 28, 2024

mostafa left a comment

Choose a reason for hiding this comment

sinadarbouy commented Jul 14, 2024 •

edited by mostafa

Loading

mostafa Jul 28, 2024 •

edited

Loading