[ZBXNEXT-6492] Possibility for usage jdbc connection string for PostgreSQL Patroni cluster Created: 2021 Feb 05 Updated: 2024 Feb 20 |
|
Status: | Reopened |
Project: | ZABBIX FEATURE REQUESTS |
Component/s: | Frontend (F), Server (S) |
Affects Version/s: | None |
Fix Version/s: | None |
Type: | Change Request | Priority: | Major |
Reporter: | Igor Gorbach (Inactive) | Assignee: | Zabbix Development Team |
Resolution: | Unresolved | Votes: | 11 |
Labels: | None | ||
Remaining Estimate: | Not Specified | ||
Time Spent: | Not Specified | ||
Original Estimate: | Not Specified |
Attachments: |
![]() ![]() |
||||
Issue Links: |
|
Description |
We're able to define the only one hostname/ip-adress into configuration: We want to have a posibility for defining multiple hostnames/ip-adresses to Zabbix everytime will connnecting to alive database in cases of failover and switchover For example use full jdbc string for connection to PostgreSQL Patroni cluster
jdbc:postgresql://127.0.0.1:1544,127.0.0.2:1544/dbname?targetServerType=master
|
Comments |
Comment by Rostislav Palivoda [ 2021 Feb 15 ] |
Why not to use https://www.pgbouncer.org/ ? |
Comment by Kris Avi [ 2022 May 11 ] |
pgbouncer being another component that could fail and might be too much of overhead to add extra machines for pgbouncers, setting up VIP between them to direct traffic towards correct pgbouncer, while in that case you already would direct traffic against vip that is assigned to readwrite node.
The whole point of supporting connectionstring would be to avoid adding extra components in the chain for HA solution. I would not even go for JDBC connectionstring, as it is coming from Java side, but since zabbix is made in PHP, then the https://www.php.net/manual/en/function.pg-connect.php function seems to support libpq and it's format https://www.postgresql.org/docs/current/libpq-connect.html In that case the connectionstring could contain all the hosts in cluster and choose the one cluster that is read-write https://www.alibabacloud.com/blog/597797 Seems to be example of how it is configured in here. Since the connection creation towards correct node is done still by the same library, then no extra components will be used and no extra links in the chain will be introduced with that change.
In current setup we have set up vip in front of PG servers and in the servers there is also HAProxy installed just to direct traffic to read-write node. It is ugly and has extra parts that could fail, but it gives the chance that DB has done failover to another node, but for some reason VIP did not follow, then the DB connections still end up at RW node.
With the connectionstring potential usecase for performance optimization could be to direct write requests towards rw node and read requests to any of the node in the whole cluster, could allow scalability on read requests, which zabbix will get more to load the values for graphs. That would probably mean that in config there could be RW connectionstring and RO connectsionstring. If RO one is not filled, RW will be used. |
Comment by Dimitri Bellini [ 2022 May 17 ] |
I would like add my vote for this request! |
Comment by Kris Avi [ 2023 Jan 24 ] |
pgBouncer is not solution in here at all. pgPool-II could be possibly used in place, but it is hassle to set up pgPool-II for this purpose and another alternative is to set up HAProxy on zabbix server hosts, that forward traffic to read-write postgres node. The HAProxy solution is currently used for me, but in there the best would be to have Layer 4 proxy mode and do health checks and RW node checks in Layer 4 as well and it is annoying if you happen to have password on the role checking for RW node.
So I took half a day and generated patch for this feature zabbix6_pg_connstring.patch
I tested PHP side and seemed to work fine, even when switchover from one node to another. Was not able to build C++ Server application to test it, since working on Windows mainly and setting up build environment currently would take longer than needed.
This patch seemed to be good for tag '6.0.0' and checked out commit was 5203d2ea7d901cd33d148f20586e2155901a7faa.
Patch implements Postgres connectionstring support and support to add target_session_attr on Postgres connections when using old parameters. In the old code it is possible to add multiple hosts on DBHost field, making it work quite similar to what connstring is implementing. But for automatic failover the target_session_attr is required. |
Comment by Kris Avi [ 2023 Mar 09 ] |
Updated patch to current master (6cfe791429d7cccda0205335868976cac6c51039) zabbix6_4_pg_connstring.patch Patch still has both changes, one to add target_session_attr config option (with default value of "read-write") and other one to use connectionstring instead of all the connection parameters. Even the target_session_attr change would give possibility to write multiple hosts on DBHost field and get correct HA capabilities. If you would like to keep old behavior, then target_session_attr could have the default value of "any", but would be good to have possibility to configure it. This change is vital for Zabbix HA capabilities, without database zabbix is nothing. It is not normal situation to change Zabbix config if for some reason DB should have to do failover to replica node. If you still suggest using something like pg_bouncer to do that failover, could you please provide sample configuration for it, cause so far I have not seen pg_bouncer having such possibility to proxy connections to R/W node. Since there has been bigger changes, then I am not so sure if new patch contains all the required changes as I still do not have build environment set up to compile zabbix_server and zabbix_proxy. If more load-balancing from DB is wanted, then going further it would be nice to configure read-only db config (by default being same as rw config) and having queries tagged as read-only or queries that modify data and then directing read-only queries towards config for read only and queries that modify data only towards RW config. That way it would be possible to spread the queries across DB cluster. At least talking from Postgres point of view when hot_standby is used, but I am sure similar setup is possible with MariaDB and Oracle. |
Comment by Piotr Paweł Stefaniak [ 2023 Mar 28 ] |
I'd very much like to see this feature in. I haven't tested the patches, but it seems to me that stripping one of the if statements of parentheses would make the translation unit incorrect. |
Comment by Antti Hurme [ 2023 Jul 21 ] |
This could also be a good addition if you use pg_auto_failover. Would like to see this as an way to make Zabbix truly HA with the great inbuild server HA + PostgreSQL HA with something modern. "Implementing client-side High Availability is included in PostgreSQL’s driver libpq from version 10 onward. Using this driver, it is possible to specify multiple host names or IP addresses in the same connection string." |
Comment by Piotr Paweł Stefaniak [ 2023 Sep 05 ] |
I was in the process of writing a patch to implement this for 7.x, but it appears that there's a work-around good enough for me and perhaps others that doesn't involve patching the sources. What I needed from my WIP patch was a couple of things:
It had already been possible to set multiple hosts in the DBHost field and it worked with Postgres as intended, but not multiple ports which I needed. The problem was that DBPort is converted to an integer and stored in a variable common to all database connection code, so I had a dilemma how to implement what I wanted but at the same time keep everybody else happy. Someone over in the IRC channel asked whether pgservices could work and I thought that it's a nice idea, but there's a similar one, but simpler: environment variables PGPORT and PGTARGETSESSIONATTRS. This may be surprising, but it appears to work, zabbix-server doesn't do anything about it and libpq handles them as designed. You'd think for consistency I should also set PGHOST or PGHOSTADDR, but if you don't set it in the zabbix-server config, it's going to default to localhost, which in my case beats the purpose of everything else mentioned here. I think ideally zabbix-server should cease trying to parse and handle all possible connection parameters for all database engines that it supports and instead expose a single configuration setting for the user to provide the connection URI. |
Comment by vl1987 [ 2023 Sep 21 ] |
In fact, it is strange that there is still no such possibility. It would be great if such an option appears |
Comment by Kris Avi [ 2024 Feb 20 ] |
Thanks @pstef I opted to use environment variable PGTARGETSESSIONATTRS to solve my problem. In configuration added multiple DB hosts. My setup was easier since used default port, but different hosts. Made similar change in PHP FPM config to pass same environment variable to web interface as well. Been using that method for couple of months in production, so far on switchover, there is minor hiccup as zabbix detects that connections should be recreated on DB switchover, but so far system has been able to behave normally. This method works only because they are not setting it to some value. My patch itself would set some default value, but at the same time give you option to set it in config. Potentially could have added multiple ports option in ports field too, but it was more complicated than to add connectionstring field. I don't bother updating my patches for it any more, big portion of work is already done in there and it is up to Zabbix guys to either use it and merge it to upstream or not. I agree with you, they should just give the option to enter one connection string field and not bother with parsing and handling them all. But for sake of backwards compatibility probably those fields should stay. However still could add connection string field and if it is entered, then not try to parse rest of DB config parameters and trust that user configuring already knows what they are doing. Not all of the users will need such option, but some might. When writing my patch I kept in mind that old way should still work, it should include config file descriptions to my best knowledge and should not break something on other DB engines, but in the end it is zabbix dev team choice to use that patch or not. So far it seems this ticket has not gotten any of their attention and will be among other unsolved bug reports. Yes there are more parameters you could set and parsing them all could be too much, but most commonly used ones are sslmode and target_session_attrs, that is why potentially those should be in config while rest could come from environment variables.
Still there could be improvements made in backend and to make zabbix server and web aware of query types and if they are read-only queries or need to modify data. If it is read-only query, then it could be loadbalanced using prefer-standby for those queries and direct write queries to read-write node. It of course brings danger of reading stale data, but that is more on DBA to handle on cluster side. |