Add Manual host banning to PgCat #340
Conversation
src/errors.rs
Outdated
| MessageReceiveFailed, | ||
| FailedCheckout, | ||
| StatementTimeout, | ||
| ManualBan, |
There was a problem hiding this comment.
| ManualBan, | |
| AdminBan, |
src/admin.rs
Outdated
| { | ||
| let host = match tokens.get(1) { | ||
| Some(host) => host, | ||
| None => return error_response(stream, "BAN command requires a hostname to ban").await, |
There was a problem hiding this comment.
non blocking, can be done in a follow up: do we want to accept a duration string for how long to ban it for?
| res.put(row_description(&columns)); | ||
|
|
||
| for (id, pool) in get_all_pools().iter() { | ||
| for address in pool.get_addresses_from_host(host) { |
There was a problem hiding this comment.
Do you think we should use the address name here instead to make this more like the other admin commands?
There was a problem hiding this comment.
The common use case for admin banning (from my experience) is when a database is in a degraded state or is about to under go some maintenance. Using hostname for admin banning in these situation makes more sense as opposed to having to do an extra lookup to figure out the address name that corresponds to the host
There was a problem hiding this comment.
As long as we show that host name somewhere in our stats, so the user can find it without guessing, e.g. sometimes people use IP addresses and sometimes they use DNS, and sometimes both refer to the same place.
src/admin.rs
Outdated
| _ => pool.settings.ban_time, | ||
| }; | ||
| let remaining = ban_duration - (now - ban_time.timestamp()); | ||
| if remaining <= 0 || address.role == Role::Primary { |
There was a problem hiding this comment.
Primary should never be added to this data structure.If it was, we may want to know.
src/admin.rs
Outdated
|
|
||
| for (id, pool) in get_all_pools().iter() { | ||
| for address in pool.get_addresses_from_host(host) { | ||
| if !pool.is_banned(&address) && address.role != Role::Primary { |
There was a problem hiding this comment.
The primary check should be handled by the pool ideally as it is now I believe?
Sometimes we want an admin to be able to ban a host for some time to route traffic away from that host for reasons like partial outages, replication lag, and scheduled maintenance. We can achieve this today using a configuration update but a quicker approach is to send a control command to PgCat that bans the replica for some specified duration. This command does not change the current banning rules like Primaries cannot be banned When all replicas are banned, all replicas are unbanned
Sometimes we want an admin to be able to ban a host for some time to route traffic away from that host for reasons like partial outages, replication lag, and scheduled maintenance.
We can achieve this today using a configuration update but a quicker approach is to send a control command to PgCat that bans the replica for some specified duration.
This command does not change the current banning rules like
Commands added