)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":28691,"name":"Bo Tran","email":"ministry.96.nd@gmail.com","username":"ministry"},"change_message_id":"8f79e2545ef8821c09e328ab6160fbde77a8a87a","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"d69cf7a0_0bd35aae","updated":"2026-01-26 03:15:01.000000000","message":"@all I think this is special case, we need to view overal. In my production, we have some of types attach configuration group with following:\n\nAssume:\nHave configuration group M - attach to master, S - attach to slave.\n\nIn configuration group M and S will have same config.\n\nThe client aka a database engineer will:\n1. just attach M to master\n2. just attach S to master\n3. attach M to master, S to slave as assume and expect nothing being override\n\nwith above cases. my production will re-apply configuration group to nodes.\nso i think, with your case, we just need delete postgresql.auto.conf before do backup or restore and when we do apply/re-apply a configuration group\n\nwhat do you think about this?","commit_id":"bb2e8276c0fe5ab281d796e4ef246cb10a2ff142"},{"author":{"_account_id":31737,"name":"Hirotaka Wakabayashi","email":"hiwkby@yahoo.com","username":"hiwkby"},"change_message_id":"530bcd7566105e4eda296deec9fa4344092132e9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"4091a985_e09ef418","updated":"2026-01-25 00:29:26.000000000","message":"Hello Bo, Could you please check this?","commit_id":"bb2e8276c0fe5ab281d796e4ef246cb10a2ff142"},{"author":{"_account_id":28691,"name":"Bo Tran","email":"ministry.96.nd@gmail.com","username":"ministry"},"change_message_id":"291296286a828899dd758d33109a523f68072681","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"ad9d7f7c_1bc2062e","in_reply_to":"20ccbacb_ae778f03","updated":"2026-01-26 09:05:19.000000000","message":"Your ideas are nice but it maybe make more compact for older versions. such as: if postgres version 19 add more config options (not being list on above) and need to be identical on both the primary and its replicas.\n\nProblem here: we need to fix this problem one more again, so I think you should review what happend when we do delete `postgresql.auto.conf` before backup/ restore, whether above config options being sync from master to replicas again and we expecting?","commit_id":"bb2e8276c0fe5ab281d796e4ef246cb10a2ff142"},{"author":{"_account_id":36080,"name":"Erkin Mussurmankulov","display_name":"Eric","email":"mangust404@gmail.com","username":"mongoose404","status":"PS Cloud services employee"},"change_message_id":"a3eae617d3345757efc1ccf9a84e63e670a39f34","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"c3c484f0_14887ef7","in_reply_to":"88dde86c_6ed0a77e","updated":"2026-01-27 03:44:17.000000000","message":"Sure, here it is.\n\nInput:\n- Configuration C1 with max_connections \u003d 100\n- Configuration C2 with max_connections \u003d 200\n- PostgreSQL primary P and replica R\n___\n\nScenario 1\n1. create P\n2. create R\n3. attach C2 to P\n4. operating_status on P\u003dRESTART_REQUIRED, operating_status on R\u003dRESTART_REQUIRED\n5. reboot P, wait for operating_status\u003dHEALTHY\n6. reboot R, wait for operating_status\u003dHEALTHY\n7. check max_connections on P and R *(should be \u003d200)*\n*(with current upstream it would be 100 and impossible to change if storage_driver\u003dcinder)*\n8. detach C2 from P\n9. operating_status on P\u003dRESTART_REQUIRED, operating_status on R\u003dRESTART_REQUIRED\n10. reboot P, wait for operating_status\u003dHEALTHY\n11. reboot R, wait for operating_status\u003dHEALTHY\n12. check max_connections on P and R *(should be \u003d100)*\n\n___\n\nScenario 2\n1. create P with C2, check max_connections\u003d200\n2. create R with C1 should raise InvalidValue\n3. create R\n4. R should be HEALTHY and has max_connections 200\n5. reboot R\n6. R should be HEALTHY and has max_connections 200\n7. try to attach C1 to R should raise InvalidValue\n8. detach C2 from P\n9. reboot P, reboot R\n10. check max_connections on P and R *(should be \u003d100)*\n11. attach C1 to R, should not raise error\n12. attach C2 to P should raise InvalidValue\n\n___\n\nThese two scenarios will cover most use cases.\n\nPlease remember that scenarios should work with any \"special\" parameter from the list, not only max_connections:\n- max_connections\n- max_prepared_transactions\n- max_locks_per_transaction\n- max_worker_processes\n- max_wal_senders\n- max_replication_slots\n- wal_level\n- track_commit_timestamp","commit_id":"bb2e8276c0fe5ab281d796e4ef246cb10a2ff142"},{"author":{"_account_id":36080,"name":"Erkin Mussurmankulov","display_name":"Eric","email":"mangust404@gmail.com","username":"mongoose404","status":"PS Cloud services employee"},"change_message_id":"fb034d3293dfc5f9233ab8a0ebc54199de5ea381","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"cc4d8ed0_89a84ce1","in_reply_to":"ad9d7f7c_1bc2062e","updated":"2026-01-26 12:01:31.000000000","message":"If we just remove workaround described here:\n\nhttps://www.postgresql.org/message-id/20220203094727.w3ca3sukfu5xu7hk%40jrouhaud\n\nThen, if you will take trove instance without configuration attached,\nreplication attachment will work fine.\n\nBut when you\u0027ll set max_connections to a different value in\nconfiguration on primary, and try to create new replica, it will know\nnothing about max_connections on the primary and replication attachment\nwill fail.\n\n\n\nLet\u0027s assume that we will just add code which will remove\npostgresql.auto.conf and leave current workaround with `max_connections`\nas it is:\n\n- user set max_connections to a different value on primary (e.g. 300)\nin configuration\n- user invoke replica create (without configuration)\n- trove will create postgresql.auto.conf and pass it within snapshot to\na replica\n- trove will attach replica successfully and then remove\npostgresql.auto.conf\n- when replica will restart, it will fail to connect to primary because it has a\ndefault max_connection \u003d 100 in config\n\n_____________________________________________________\n\nSo we still need to find a solution for this issue...\n\nIn addition to my proposed solution from the previous message:\nFor not hard-coding a list of parameters in the code, we can mark\nparameters with a flag in validation-rules, e.g.\n``\"replication_sync\": True|False``\n\nWhen new parameter will appear in future versions, it can be easily\nmanaged by validation rules. Also this functionality may be applied for\nanother datastore managers.\n\nYes, my solution seems complex, but the problem itself is not\ntrivial but quite important 😞\n\nProbably there is another simple solution which I may not see.","commit_id":"bb2e8276c0fe5ab281d796e4ef246cb10a2ff142"},{"author":{"_account_id":28691,"name":"Bo Tran","email":"ministry.96.nd@gmail.com","username":"ministry"},"change_message_id":"b664ba4ae290eab1fc6ca120bb4bcce0da880f50","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"88dde86c_6ed0a77e","in_reply_to":"cc4d8ed0_89a84ce1","updated":"2026-01-27 02:17:54.000000000","message":"yeah, I will lab about this problem and we will discuss solutions later. Can you share all use cases that you known for me?","commit_id":"bb2e8276c0fe5ab281d796e4ef246cb10a2ff142"},{"author":{"_account_id":36080,"name":"Erkin Mussurmankulov","display_name":"Eric","email":"mangust404@gmail.com","username":"mongoose404","status":"PS Cloud services employee"},"change_message_id":"e5e8e9d540e3d9f9d142e0f5c24d056b5233cec5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":3,"id":"20ccbacb_ae778f03","in_reply_to":"d69cf7a0_0bd35aae","updated":"2026-01-26 07:20:13.000000000","message":"Hello, Bo! Thank you for the reply.\n\nYes, I agree that deleting ``postgresql.auto.conf`` at certain points may mitigate the incorrect behavior of ``max_connections`` management via configuration groups. However, we still need to properly handle the replica-primary attachment process.\n\nIf a user attempts to create a replica and it ends up in the ERROR operational state, this should be considered as a bug in Trove.\n\nBelow is a list of parameters that must be identical on both the primary and its replicas:\n- ``max_connections``\n- ``max_prepared_transactions``\n- ``max_locks_per_transaction``\n- ``max_worker_processes``\n- ``max_wal_senders``\n- ``max_replication_slots``\n- ``wal_level``\n- ``track_commit_timestamp``\n\nMy original proposal implemented in this patch - automatic synchronization of configuration between the replica and the primary - is clearly insufficient for cases where the primary and replicas start with different configurations.\n\nAn alternative approach would be the following:\n\nThe parameters listed above should be managed by Trove and always kept in sync with the primary. The design of ``postgresql.auto.conf`` makes this feasible.\n\nTo implement this, we would need to:\n\n1. Keep and extend the existing code that generates ``postgresql.auto.conf`` by adding all parameters from the list above (this likely requires a dedicated helper method).\n\n2. Add logic to the PostgreSQL manager that, during the replica attach process, checks whether any of these \"special\" parameters are set in the configuration and handles them accordingly.\n\n3. To handle these special parameters correctly, they should be written to ``postgresql.auto.conf``. The simplest way to propagate them to replicas is to execute ALTER SYSTEM SQL commands on each attached replica from primary (we need additional checks if this will work).\n\n4. Add documentation for DBAs describing these \"special\" parameters and how they are managed by Trove.\n\nWhat do you think about this design?","commit_id":"bb2e8276c0fe5ab281d796e4ef246cb10a2ff142"}],"trove/guestagent/datastore/postgres/manager.py":[{"author":{"_account_id":31737,"name":"Hirotaka Wakabayashi","email":"hiwkby@yahoo.com","username":"hiwkby"},"change_message_id":"421e10cdf7589a2e1d8892f6d9d8571a0dbf6acc","unresolved":true,"context_lines":[{"line_number":345,"context_line":"            max_connections \u003d f\u0027max_connections\u003d{result}\u0027"},{"line_number":346,"context_line":"            operating_system.write_file(autoconf_file,"},{"line_number":347,"context_line":"                                        max_connections, as_root\u003dTrue)"},{"line_number":348,"context_line":""},{"line_number":349,"context_line":"        _start_backup()"},{"line_number":350,"context_line":""},{"line_number":351,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":3,"id":"a51a5441_f1e4c296","side":"PARENT","line":348,"updated":"2026-01-23 08:56:37.000000000","message":"Hello, thanks for the PR! I think it would be more semantically correct \nto use \"ALTER SYSTEM SET max_connections\u003d{maxcons};\" or something here. What do you think?","commit_id":"0243ca14069331630d94a5dfd8a42f6eb64ea05e"},{"author":{"_account_id":36080,"name":"Erkin Mussurmankulov","display_name":"Eric","email":"mangust404@gmail.com","username":"mongoose404","status":"PS Cloud services employee"},"change_message_id":"1ad630d7f5313185d1cfea1242aae6337c545fce","unresolved":false,"context_lines":[{"line_number":345,"context_line":"            max_connections \u003d f\u0027max_connections\u003d{result}\u0027"},{"line_number":346,"context_line":"            operating_system.write_file(autoconf_file,"},{"line_number":347,"context_line":"                                        max_connections, as_root\u003dTrue)"},{"line_number":348,"context_line":""},{"line_number":349,"context_line":"        _start_backup()"},{"line_number":350,"context_line":""},{"line_number":351,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":3,"id":"8f92ad5c_fd96db88","side":"PARENT","line":348,"in_reply_to":"a51a5441_f1e4c296","updated":"2026-01-24 13:28:38.000000000","message":"Hello, Hirotaka! Thank you for the review.\n\nThe command ``\"ALTER SYSTEM SET max_connections\u003d{maxcons};\"`` does the same thing: it writes ``max_connections`` to the postgresql.auto.conf file (by the PostgreSQL server process itself). This leads to the main issue: you will no longer be able to set ``max_connections`` using Trove’s configuration mechanism.\n\nIn general, setting any option using ALTER SYSTEM or by writing it directly into postgresql.auto.conf causes this effect: the option can no longer be controlled via configuration.\n\nConsidering this, the purpose of the original workaround (which I propose to remove) was to synchronize the ``max_connections`` value between the primary and the replica. It may have made sense at some point, because in addition to ``max_connections``, there are several options that must not differ when a replica attaches to a primary.\n\nThe proposed solution is to apply the same configuration to the replica if it is present on the primary. If ``max_connections`` is not set in the configuration, or if there is no attached configuration at all, we assume that the templates on both servers are the same, and replica attachment will work correctly in all cases.\n\nThis workaround was originally added here:\nhttps://review.opendev.org/c/openstack/trove/+/903440\n\nI was not able to reproduce any conditions in which this workaround actually fixes a real problem, and I am not sure why it was added. However, it does break the configuration logic.\n\nWe should probably ask Bo whether this patch is still relevant and what he thinks about my solution. Does anyone know if he is still working on Trove?","commit_id":"0243ca14069331630d94a5dfd8a42f6eb64ea05e"}]}