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

Feature/support pgbouncer #1400

Closed
wants to merge 4 commits into from

Conversation

jsvisa
Copy link
Contributor

@jsvisa jsvisa commented Jun 22, 2016

  • CHANGELOG.md updated
  • README.md updated

@jsvisa jsvisa force-pushed the feature/support-pgbouncer branch 5 times, most recently from 0de78bd to 5082598 Compare June 23, 2016 01:21
@sparrc
Copy link
Contributor

sparrc commented Jul 14, 2016

what does this add that the regular postgres plugin isn't able to provide?

@sparrc
Copy link
Contributor

sparrc commented Aug 9, 2016

closing for lack of updates

@sparrc sparrc closed this Aug 9, 2016
@jsvisa
Copy link
Contributor Author

jsvisa commented Aug 9, 2016

@sparrc Sorry for the delay.

Yes, the postgres plugin can provide the detail.

For PgBouncer, all statistics are stored in one database, default as 'pgbouncer', and the backend databases' information are stored in it as regular records. See more from PgBouncer usage.

For telegraf/postgresql.go at master · influxdata/telegraf, the command not applied,

and for telegraf/postgresql_extensible.go at master · influxdata/telegraf, it always need version information

@sparrc
Copy link
Contributor

sparrc commented Aug 9, 2016

For telegraf/postgresql.go at master · influxdata/telegraf, the command not applied,

what does that mean exactly? can you add "the command" to that plugin?

and for telegraf/postgresql_extensible.go at master · influxdata/telegraf, it always need version information

please also explain more, what do you mean that it always needs version information? why is pgbouncer not compatible with "version information"?

@jsvisa
Copy link
Contributor Author

jsvisa commented Aug 9, 2016

Sorry for my rudeness. First of all, pgbouncer/pgbouncer: PgBouncer development is a

Lightweight connection pooler for PostgreSQL.

Pgbouncer always store the stat info inside it's own datebase, not in PostgreSQL's side.
The stat of PostgreSQL not applied to pgbouncer.

And in postgresql_extensible.go, we need to fetch the PostgreSQL's version first, then execute the specified query string, but the version query

query = select substring(setting from 1 for 3) as version from pg_settings where name='server_version_num'

is not available in the special administration database pgbouncer , which only support below commands:

SHOW [HELP|CONFIG|DATABASES|FDS|POOLS|CLIENTS|SERVERS|SOCKETS|LISTS|VERSION]
SET key = arg
RELOAD
PAUSE
SUSPEND
RESUME
SHUTDOWN

So I think we need add the pgbouncer plugin along with postgresql and postgresql_extension.

@sparrc
Copy link
Contributor

sparrc commented Aug 9, 2016

okay, fair enough, thanks for the clarification

@jsvisa
Copy link
Contributor Author

jsvisa commented Aug 9, 2016

May I reopen this PR?

@sparrc
Copy link
Contributor

sparrc commented Aug 10, 2016

yes, sorry, I meant to do that with the last comment

@sparrc sparrc reopened this Aug 10, 2016
@jsvisa
Copy link
Contributor Author

jsvisa commented Aug 10, 2016

@sparrc thanks, rebased

@sparrc
Copy link
Contributor

sparrc commented Aug 10, 2016

Can you explain why you've used your own docker image for the tests? From googling there appears to be some others that are more common/popular. I ask because I've had contributors add docker images that have gone defunct.

@jsvisa
Copy link
Contributor Author

jsvisa commented Aug 10, 2016

Mine is based on alpine as base image, which is smaller(only 16MB), and seems the others use Ubuntu, which size almost is 98MB.

@sparrc sparrc closed this in fec9760 Aug 10, 2016
@jsvisa jsvisa deleted the feature/support-pgbouncer branch August 10, 2016 14:18
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

Successfully merging this pull request may close these issues.

None yet

2 participants