Skip to content

Make queue strategy configurable and default to Fifo #463

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Jun 9, 2023
Merged

Make queue strategy configurable and default to Fifo #463

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
075b8a8
Change idle timeout default to 10 minutes
zainkabani May 31, 2023
01e9145
Revert lifo for now while we investigate connection thrashing issues
zainkabani May 31, 2023
ddaab1a
Make queue strategy configurable
zainkabani Jun 1, 2023
3d15710
test revert idle time out
zainkabani Jun 1, 2023
9b6b6c5
Add pgcat start to python test
zainkabani Jun 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Make queue strategy configurable
  • Loading branch information
@zainkabani
zainkabani committed Jun 1, 2023
commit ddaab1aba56a5575b9c12140666ee5ba6d0bf13b
11 changes: 10 additions & 1 deletion src/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,9 @@ pub struct General {
#[serde(default = "General::default_server_lifetime")]
pub server_lifetime: u64,

#[serde(default = "General::default_server_round_robin")] // False
pub server_round_robin: bool,

#[serde(default = "General::default_worker_threads")]
pub worker_threads: usize,

Expand Down Expand Up @@ -390,6 +393,10 @@ impl General {
pub fn default_prometheus_exporter_port() -> i16 {
9930
}

pub fn default_server_round_robin() -> bool {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This brings us back to original PgCat behavior before the bb8 queuing strategy change, is that correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's correct, using a queue instead of stack as the default. Once we figure out if this is causing connection thrashing we can switch the default and match pgbouncer

true
}
}

impl Default for General {
Expand Down Expand Up @@ -424,7 +431,8 @@ impl Default for General {
auth_query: None,
auth_query_user: None,
auth_query_password: None,
server_lifetime: 1000 * 3600 * 24, // 24 hours,
server_lifetime: Self::default_server_lifetime(),
server_round_robin: false,
validate_config: true,
}
}
Expand Down Expand Up @@ -983,6 +991,7 @@ impl Config {
"Default max server lifetime: {}ms",
self.general.server_lifetime
);
info!("Sever round robin: {}", self.general.server_round_robin);
match self.general.tls_certificate.clone() {
Some(tls_certificate) => {
info!("TLS certificate: {}", tls_certificate);
Expand Down
8 changes: 7 additions & 1 deletion src/pool.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use arc_swap::ArcSwap;
use async_trait::async_trait;
use bb8::{ManageConnection, Pool, PooledConnection};
use bb8::{ManageConnection, Pool, PooledConnection, QueueStrategy};
use bytes::{BufMut, BytesMut};
use chrono::naive::NaiveDateTime;
use log::{debug, error, info, warn};
Expand Down Expand Up @@ -389,6 +389,11 @@ impl ConnectionPool {
.min()
.unwrap();

let queue_strategy = match config.general.server_round_robin {
true => QueueStrategy::Fifo,
false => QueueStrategy::Lifo,
};

debug!(
"[pool: {}][user: {}] Pool reaper rate: {}ms",
pool_name, user.username, reaper_rate
Expand All @@ -401,6 +406,7 @@ impl ConnectionPool {
.idle_timeout(Some(std::time::Duration::from_millis(idle_timeout)))
.max_lifetime(Some(std::time::Duration::from_millis(server_lifetime)))
.reaper_rate(std::time::Duration::from_millis(reaper_rate))
.queue_strategy(queue_strategy)
.test_on_check_out(false);

let pool = if config.general.validate_config {
Expand Down