search

cardano-community / guild-operators

Artifacts and scripts created by Guild operators
https://cardano-community.github.io/guild-operators
MIT License
354 stars 178 forks source link

Grest: Cron job names in `kill_cron_psql_process()` do not match sql procedure names #1666

Closed xray-robot closed 11 months ago

xray-robot commented 1 year ago

The names are not the same as the function/procedure names in sql. So there is no tasks interruption on remove_all_grest_cron_jobs() because grest.get_query_pids_partial_match doesn't find the right PID, and reinitialization freezes if cron jobs not completed:

https://github.com/cardano-community/guild-operators/blob/ae49c9707852413ac8fdc15f95dd3bdfb5c5ce4d/scripts/grest-helper-scripts/setup-grest.sh#L223-L225

Solution: it may be worth rename procedures and functions according to the names from remove_all_grest_cron_jobs():

rdlrt commented 1 year ago

Alternative proposal - remove possibility for any cron job or interactive psql-based job to be running by using something like below in setup-grest.sh instead of listing every cron job to kill: psql cexplorer -c 'select pg_cancel_backend(pid) from pg_stat_activity where usename='"'"'${USER}'"'"' and application_name = '"'"'psql'"'"' and query not like '"'"'%pg_stat_activity%'"'"';'

As any DB Pool connection (incl. DBSync/PostgREST) queries do not register application_name as psql

Need to test to ensure there are no edge cases. If there is a worry about customized use-cases, we can also add query like '"'"'grest'"'"' to the command

xray-robot commented 1 year ago

I think these functions must be extended as you wrote:

https://github.com/cardano-community/guild-operators/blob/ae49c9707852413ac8fdc15f95dd3bdfb5c5ce4d/scripts/grest-helper-scripts/db-scripts/basics.sql#L114

https://github.com/cardano-community/guild-operators/blob/ae49c9707852413ac8fdc15f95dd3bdfb5c5ce4d/scripts/grest-helper-scripts/db-scripts/basics.sql#L133

rdlrt commented 1 year ago

The point is - if we go with my proposal we would be able to get rid of much of those functions and replace the logic entirely 🙂

xray-robot commented 1 year ago

The point is - if we go with my proposal we would be able to get rid of much of those functions and replace the logic entirely 🙂

Yeah, I just wanted to point that out, maybe these functions are being used elsewhere. I don't know if I should close this issue or you will follow up on it. :)

rdlrt commented 1 year ago

Ah - we will test out and link a PR to map against commit