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

Update ddl.sql #469

Closed
wants to merge 1 commit into from
Closed

Update ddl.sql #469

wants to merge 1 commit into from

Conversation

hellower
Copy link

@hellower hellower commented Aug 7, 2022

client_pid(worker_pid)

The server running pg_timetable may be different, but coincidentally the pid may be the same.
At my docker enviroment, serveral same pid(differnt docker constainer) happended.

Duplicate execution can be prevented by adding the client_addr column as shown below.

SQL> select * from timetable.active_sessions
client_id


14 pg_timetable 28417 167.71.214.34 2022-08-07 14:12:24.333 +0900
14 pg_timetable 28418 167.71.214.34 2022-08-07 14:12:24.366 +0900
14 pg_timetable 28422 128.199.132.200 2022-08-07 14:12:29.563 +0900
14 pg_timetable 28423 128.199.132.200 2022-08-07 14:12:29.589 +0900

Copy link
Collaborator

@pashagolub pashagolub left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. It makes sense. On the other hand, these issues must be addressed before:

  • update timetable.log table, since it holds the pid column as well;
  • same goes for timetable.execution_log;
  • all changes to the DB schema should be implemented as a new migration

@@ -187,15 +188,15 @@ BEGIN
PERFORM 1
FROM timetable.active_session s
WHERE
s.client_pid <> worker_pid
s.client_addr <> inet_client_addr()
Copy link
Collaborator

Choose a reason for hiding this comment

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

What if we run several instances on the same host? Then inet client addr will be the same

Copy link
Author

@hellower hellower Aug 8, 2022

Choose a reason for hiding this comment

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

s.client_addr <> inet_client_addr()
AND s.client_name = worker_name # !!!!

If there are multiple instances on the same host, each clientname must be unique.

............ config.yaml 
# clientname:                    Unique name for application instance
clientname: pg_scheduler

..... bootstrap.go 
sql := fmt.Sprintf("SELECT timetable.try_lock_client_name(%d, $worker$%s$worker$)", os.Getpid(), pge.ClientName)

Copy link
Collaborator

Choose a reason for hiding this comment

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

The client name must be absolutely unique, even if your workers are running on different hosts.

Copy link
Collaborator

Choose a reason for hiding this comment

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

And that, in turn, means we have a bug that must be fixed immediately. 🤔

Copy link
Collaborator

@pashagolub pashagolub Aug 15, 2022

Choose a reason for hiding this comment

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

To prove this fix won't prevent a bug, you can try to start several docker containers, e.g.

$ docker run -d --rm cybertecpostgresql/pg_timetable:latest postgresql://<conn_str> -c worker001
5b1ab3ad6bd585db05f88130bb78f1418ac57ab3b1c8a755703017dc4ff486da

$ docker run -d --rm cybertecpostgresql/pg_timetable:latest postgresql://<conn_str> -c worker001
4de31c35c0226d89ef9100324e611e7caad27a9ac30fc588c54263036597137f

$ docker run -d --rm cybertecpostgresql/pg_timetable:latest postgresql://<conn_str> -c worker001
b0767832353589529d69f36f04b9ee14a25d14d8e796b8bf0bbae467a5e280de

$ psql

timetable=>SELECT *
FROM timetable.active_session s
        WHERE
            s.client_name = 'worker001';

client_pid|client_name|server_pid|started_at                   |
----------+-----------+----------+-----------------------------+
         1|worker001  |     27596|2022-08-15 14:12:39.333 +0200|
         1|worker001  |     26052|2022-08-15 14:12:39.565 +0200|
         1|worker001  |     24348|2022-08-15 14:12:58.131 +0200|
         1|worker001  |     18848|2022-08-15 14:12:58.245 +0200|
         1|worker001  |     26692|2022-08-15 14:40:06.516 +0200|
         1|worker001  |      8620|2022-08-15 14:40:06.713 +0200|

client_pid(worker_pid)

The server running pg_timetable may be different, but coincidentally the pid may be the same.

Duplicate execution can be prevented by adding the client_addr column as shown below.

---------  -------------------------------------------------------------------
SQL> select * from timetable.active_sessions
client_id 
---------  -------------------------------------------------------------------
14	pg_timetable	28417	167.71.214.34	2022-08-07 14:12:24.333 +0900
14	pg_timetable	28418	167.71.214.34	2022-08-07 14:12:24.366 +0900
14	pg_timetable	28422	128.199.132.200	2022-08-07 14:12:29.563 +0900
14	pg_timetable	28423	128.199.132.200	2022-08-07 14:12:29.589 +0900
@pashagolub
Copy link
Collaborator

Thanks for your help! Superseded by #476

@pashagolub pashagolub closed this Aug 16, 2022
@hellower
Copy link
Author

Thank you!

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