chore: gdb regions improvements - #679
Conversation
9af7ec7 to
28c68b9
Compare
28c68b9 to
69e60fd
Compare
| return undefined; | ||
| } | ||
|
|
||
| async filterAvailableHosts(hosts: HostInfo[], accessibleRegions: string[]): Promise<HostInfo[]> { |
There was a problem hiding this comment.
Question: since the method is optional do we even need the implementations for plain mysql and pg dialects? Does it help performance wise to have them? Bc my understanding is that when this is not implemented it has the same behavior as returning all hosts
| ); | ||
| } | ||
|
|
||
| async filterAvailableHosts(hosts: HostInfo[], accessibleRegions: string[]): Promise<HostInfo[]> { |
There was a problem hiding this comment.
We only call this once it seems, from the failover plugins, I'm curious why we have the same method defined in two dialects that doesn't seem to be dialect independent - can't we handle this filtering in a shared location?
| if (!this.accessibleRegions) { | ||
| return hosts; | ||
| } | ||
| return hosts.filter((host) => { |
There was a problem hiding this comment.
I feel like this is similar logic to what is in the dialects - we can probably handle it in the accessible_regions file in utils
| "or specific regions. Possible values include: strict-writer-primary, strict-writer-secondary, " + | ||
| "strict-reader-primary, strict-reader-secondary, writer-or-reader-primary, writer-or-reader-secondary, " + | ||
| "or a specific AWS region name.", | ||
| null |
There was a problem hiding this comment.
Shouldn't the default be strict-writer-primary
| const lowerRegions = accessibleRegions.map((r) => r.toLowerCase()); | ||
| return hosts.filter((host) => { | ||
| const region = this.rdsUtils.getRdsRegion(host.host); | ||
| return region !== null && lowerRegions.includes(region.toLowerCase()); |
There was a problem hiding this comment.
| return region !== null && lowerRegions.includes(region.toLowerCase()); | |
| return region !== null && lowerRegions.includes(region); |
Pretty sure we put it into lower case accesible_regions parsing
| this.accessibleRegions = AccessibleRegions.parse(properties); | ||
|
|
||
| if (this.accessibleRegions) { | ||
| logger.debug(`GlobalAuroraTopologyMonitor: accessible regions = ${this.accessibleRegions.join(",")}`); |
There was a problem hiding this comment.
Should this be stored in messages?
2 participants
Summary
Description
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.