telegraf icon indicating copy to clipboard operation
telegraf copied to clipboard

postgresql_extensible: max_idle and max_open params should not be freely configurable

Open redbaron opened this issue 2 years ago • 4 comments

Use Case

Both postgresql_extensible and postgresql ever only use one connection at the time, for that reason narrow subset of all possible values of max_idle and max_open make sense.

max_idle

max_idle configures how many connections to keep around in the pool between Gather. Only valid choices are 0 and 1 . When 0 Telegraf will establish connection every Gather, when 1 it will keep it around and reuse.

max_open

max_open sets how many open connections there can be. Given that there is only one connection in use, this config option should be present as only sensible value is 1.

Expected behavior

Given the narrow range of possible values for these parameters, I think it makes sense to either clamp them or deprecate and replace with:

  • max_idle int -> reuse_connection bool
  • max_open -> remove entirely and ignore value if set.

Actual behavior

presence of these params give false impression that they influence anything, but in fact they are mostly not.

Additional info

No response

redbaron avatar Aug 22 '23 19:08 redbaron

@redbaron are you referring to the options in inputs/postgresql/service.go? If so, those options are also used by inputs.pgbouncer but I guess it's the same there!?

Usually the go SQL framework works with connection pools and internally manages which connections to use but I would agree that all three implementations will only use one connection at a time... Long story short, I'm not against creating a PR for this feature if (and only if) we are sure that we do not want to extend address to take a list of servers...

srebhan avatar Aug 30 '23 16:08 srebhan

Hello! I am closing this issue due to inactivity. I hope you were able to resolve your problem, if not please try posting this question in our Community Slack or Community Forums or provide additional details in this issue and reqeust that it be re-opened. Thank you!

telegraf-tiger[bot] avatar Sep 19 '23 18:09 telegraf-tiger[bot]

we are sure that we do not want to extend address to take a list of servers...

Even if it was a list of servers, then pool number of connections config option still adds no value: there'll be a pool per server with one connection max.

redbaron avatar Sep 19 '23 20:09 redbaron

@redbaron - were you going to put up a PR with your suggested changes? I have not looked deeply or understood the historical reasons for those options, but your proposal sounds sane.

powersj avatar Sep 20 '23 19:09 powersj