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

Topology query needs review #615

Closed
davecramer opened this issue Aug 24, 2023 · 4 comments
Closed

Topology query needs review #615

davecramer opened this issue Aug 24, 2023 · 4 comments

Comments

@davecramer
Copy link
Contributor

https://github.com/awslabs/aws-advanced-jdbc-wrapper/blob/3062421215670f252842a0823ec30b285e353663/wrapper/src/main/java/software/amazon/jdbc/dialect/AuroraPgDialect.java#L40

The following query should be considered
select server_id, case when session_ID = 'MASTER_SESSION_ID' then true else false end as is_writer, cpu, coalesce(replica_lag_in_msec, 0) as lag, coalesce(last_update_timestamp, now() - make_interval(secs => coalesce(replica_lag_in_msec, 0) / 1000)) as last_update from aurora_replica_status() order by is_writer desc, last_update desc;

The problem is that the writer is the single source of truth. Since readers always lag this means that they are less reliable than the writer. This query adds the lag into the last_updated timestamp.

@aaron-congo
Copy link
Contributor

aaron-congo commented Sep 4, 2024

Hi @davecramer, I've spent a bit of time investigating this and am trying to determine if we want to make these changes or not. Based on the way that our code uses the topology information, I don't think the suggested change will have much of an effect (we don't use the resulting order or last update time for much). Consequently, I'd like to understand the desired outcome better. Based on your last sentence, I believe the intended outcome was to prioritize using the writer to fetch topology information, since it is more accurate. Is this correct?

With the current codebase, we always use the connection indicated by the customer to fetch topology, whatever instance that happens to be. However, we are currently working on a new host list provider that will always fetch topology from the writer instance, because it is more accurate. Let me know if this achieves the desired outcome of this issue or if I've misunderstood. Thanks!

@davecramer
Copy link
Contributor Author

@aaron-congo sorry for the very late response.
Yes, we want to use the writer to fetch topology information.

@aaron-congo
Copy link
Contributor

Gotcha, with the failover2 plugin that was recently added we use the writer to fetch topology info since it is the most reliable. Given that the new host list provider from this PR provides this functionality, are you okay with me closing this ticket?

@davecramer
Copy link
Contributor Author

Sure

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

No branches or pull requests

2 participants