Add DB activity based routing

[nadavshatz]

Hi all,

This is an attempt to add support for primary-replica routing based on recent writes on the table level.

We've implemented this as a separate process alongside PgPool-II before.
This will perform much better as it's incorporated into PgCat itself.

You can read all about the approach here: https://medium.com/tailor-tech/using-database-read-replicas-in-distributed-systems-d80eaf6bbf8a

[drdrsh]

I have gotten around to testing the PR. It works as described. I have a few notes

• There is an inconsistency in the units used for TTL, we should reflect the units being used in the name of the config and variables to avoid putting milliseconds in a second field.. I think unspecified unit name should mean seconds, otherwise unit should be specified
• Documentation isn't accurate as you need query_parser_enabled as well as query_parser_read_write_splitting to see the correct routing behavior

[drdrsh]

Another related note but not strictly necessary as part of this PR, is about propagation of this information to other PgCat instances in a HA deployment. I don't think it is necessary but I was curious if you had thoughts on this.

[nadavshatz]

Thanks for the detailed review!

Changes:

• I've synced the fork and rebased over main.
• I've updated the configs that are millisecond based to have ms in the their name - I completely agree with you on this.
• I've updated the config docs about the required enabled options for this to work.

HA Note:
I mention this in the considerations section in the config.md file when enabling this feature. As a whole as mentioned in the article i linked to - you're right, it's an issue. One way to resolve this is to have a centralized cache for the activity but I think that from a performance and reliability standpoint i'd prefer to use sticky sessions and get all of the same DB's client to use the same PGCat instance (we've achieved this with istio).
I also didn't want to add any kind of dependency on an external cache.
There are probably many other way's to resolve this but this has been the best tradeoff i've come up with to get this great feature with limited impact on reliability or performance.

Would love to get this merged - let me know if there's anything else I should do here.

[drdrsh]

That makes sense.

I ran the tests and clippy is complaining about

error: very complex type used. Consider factoring parts into `type` definitions
   --> src/query_router.rs:617:10
    |
617 |     ) -> Option<(Vec<Expr>, Vec<Vec<Ident>>, Option<&'a Vec<Assignment>>)> {
    |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#type_complexity
    = note: `-D clippy::type-complexity` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(clippy::type_complexity)]`

error: could not compile `pgcat` (lib) due to 1 previous error
warning: build failed, waiting for other jobs to finish...
error: could not compile `pgcat` (lib test) due to 1 previous error

Exited with code exit status 101

[nadavshatz]

Not sure why clippy didn't show me this issue 🤔
anyway - updated accordingly 🙏🏼

[rajeshsaluja]

@nadavshatz - Curious to know if table_mutation_cache_ms_ttl is working as expected for you? I'm observing stale data in the current session after writes, unless pg_sleep(0.05) or SET SERVER ROLE TO 'primary' is used to force updated reads in the current session post immediate write.

[nadavshatz]

due to some deployment issues with pgcat we haven't been able to fully use the feature yet. so i'm afraid i don't have much insight here.

[rajeshsaluja]

Got it . Thank you for the response @nadavshatz