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

allow to use application name for upgrade #915

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sylvainOL
Copy link

Instead of using replica IP address, allow to use replica name in case IP address is not reliable (like when using a service mesh).

This trigger is controller by using an environment variable (USE_APPLICATION_NAME_IN_UPGRADE) and default behavior is the previous one.

closes zalando/postgres-operator#1629

@sylvainOL
Copy link
Author

Hello @hughcapet, is there any chances to review it?

Thanks!

@sylvainOL
Copy link
Author

Hello @jopadi @idanovinda @hughcapet @Jan-M @sdudoladov,

Can you tell me what you think of this PR?

it would help a lot when postgres is used on top of service mesh such as istio

thanks!

@Jan-M
Copy link
Member

Jan-M commented Nov 6, 2023

why not query both fields, and then try depending on env var to use the application name or fall back to ip. i am not such a big fan of templating those parts of sql queries.

@Jan-M
Copy link
Member

Jan-M commented Nov 6, 2023

But yes, we will keep an eye on this, if this helps in service mesh world we should help you.

@sylvainOL
Copy link
Author

Hello @Jan-M,

first thanks for quick reply!

why not query both fields, and then try depending on env var to use the application name or fall back to ip. i am not such a big fan of templating those parts of sql queries.

I'm not (at all) an expert of SQL (and SQL in python) ^_^ so I did the best I could :)

I'm not sure on how to retrieve the values after the requests:

I should propose something like this and it would work?:

        streaming = {a: l for a, l in self.postgresql.query(
            ("SELECT client_addr, application_name, pg_catalog.pg_{0}_{1}_diff(pg_catalog.pg_current_{0}_{1}(),"
             " COALESCE(replay_{1}, '0/0'))::bigint FROM pg_catalog.pg_stat_replication")
            .format(self.postgresql.wal_name, self.postgresql.lsn_name))}

        def ensure_replica_state(member):
            ip = member.conn_kwargs().get('host')
            lag = streaming.get(ip)
            if os.getenv('USE_APPLICATION_NAME_IN_UPGRADE'):
                lag = streaming.get(member.name)
            if lag is None:
                return logger.error('Member %s is not streaming from the primary', member.name)

Instead of using replica IP address, allow to use replica name in case IP address is not reliable (like when using a service mesh).
This trigger is controller by using an environment variable and default behavior is the previous one

Signed-off-by: Sylvain Desbureaux <[email protected]>
@sylvainOL
Copy link
Author

hi @Jan-M,

I tried to apply your comment on the PR :)

@greyt75
Copy link

greyt75 commented Mar 19, 2024

Another issue we are seeing with istio is that when the COPY TO PROGRAM is executed for rsync it is sending the the istio 127.0.0.6 ip for PRIMARY_IP

@@ -172,13 +172,15 @@ def ensure_replicas_state(self, cluster):
"""
self.replica_connections = {}
streaming = {a: l for a, l in self.postgresql.query(
("SELECT client_addr, pg_catalog.pg_{0}_{1}_diff(pg_catalog.pg_current_{0}_{1}(),"
("SELECT client_addr, application_name, pg_catalog.pg_{0}_{1}_diff(pg_catalog.pg_current_{0}_{1}(),"
Copy link

Choose a reason for hiding this comment

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

This does not work. The result it put in a dictionary, so it expects to unpack two elements, not three. You need to select either client_addr or application_name.

@@ -172,13 +172,15 @@ def ensure_replicas_state(self, cluster):
"""
self.replica_connections = {}
streaming = {a: l for a, l in self.postgresql.query(
("SELECT client_addr, pg_catalog.pg_{0}_{1}_diff(pg_catalog.pg_current_{0}_{1}(),"
("SELECT client_addr, application_name, pg_catalog.pg_{0}_{1}_diff(pg_catalog.pg_current_{0}_{1}(),"
" COALESCE(replay_{1}, '0/0'))::bigint FROM pg_catalog.pg_stat_replication")
.format(self.postgresql.wal_name, self.postgresql.lsn_name))}

def ensure_replica_state(member):
ip = member.conn_kwargs().get('host')
lag = streaming.get(ip)
Copy link

Choose a reason for hiding this comment

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

This should be in the else of the next if.

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.

Client_addr on pg_catalog.pg_stat_replication wrong ip address - istio enabled
4 participants