Skip to content

Fix/rewrite jobs fqn locks#2067

Merged
wslulciuc merged 3 commits into
mainfrom
fix/rewrite_jobs_fqn_locks
Aug 10, 2022
Merged

Fix/rewrite jobs fqn locks#2067
wslulciuc merged 3 commits into
mainfrom
fix/rewrite_jobs_fqn_locks

Conversation

@collado-mike

Copy link
Copy Markdown
Collaborator

Problem

The rewrite_jobs_fqn_table function is inadvertently updating jobs even when no metadata about the job has changed. Under load, this causes significant locking issues, as the jobs_fqn table must be locked for every job update.

Solution

I updated the function to only update the table if the job is a new record or if the symlink_target_uuid is distinct from the previous value. These graphs show the number of blocked transactions and commits over time before and after the change. After applying the change (around 20:10 on these graphs), the number of commits goes way up and the number of active/blocked transactions drops.

Screen Shot 2022-08-10 at 12 08 17 PM

I also added a test assertion to validate the update of the jobs_fqn table.

Note: All database schema changes require discussion. Please link the issue for context.

Checklist

  • You've signed-off your work
  • Your changes are accompanied by tests (if relevant)
  • Your change contains a small diff and is self-contained
  • You've updated any relevant documentation (if relevant)
  • You've updated the CHANGELOG.md with details about your change under the "Unreleased" section (if relevant, depending on the change, this may not be necessary)
  • You've versioned your .sql database schema migration according to Flyway's naming convention (if relevant)
  • You've included a header in any source code files (if relevant)

…ymlinked jobs

Signed-off-by: Michael Collado <collado.mike@gmail.com>
…othing has changed

Signed-off-by: Michael Collado <collado.mike@gmail.com>
@collado-mike collado-mike requested a review from wslulciuc August 10, 2022 19:15
RETURNING uuid INTO job_uuid;
IF TG_OP = 'INSERT' OR
(TG_OP = 'UPDATE' AND OLD.symlink_target_uuid IS DISTINCT FROM NEW.symlink_target_uuid) THEN
-- update the symlink target if null. otherwise, keep the old value

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ok, the comments here are much appreciated ❤️

@wslulciuc wslulciuc left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@collado-mike thanks for the fix, and the write up and tests that went along with the change 💯 🥇

@wslulciuc wslulciuc enabled auto-merge (squash) August 10, 2022 21:27
@wslulciuc wslulciuc merged commit 476e472 into main Aug 10, 2022
@wslulciuc wslulciuc deleted the fix/rewrite_jobs_fqn_locks branch August 10, 2022 21:32
jonathanpmoraes referenced this pull request in nubank/NuMarquez Feb 6, 2025
* Update rewrite_jobs_fqn_table function to correctly ignore existing symlinked jobs

Signed-off-by: Michael Collado <collado.mike@gmail.com>

* Update rewrite_jobs_fqn_table to avoid updating jobs_fqn table when nothing has changed

Signed-off-by: Michael Collado <collado.mike@gmail.com>

Signed-off-by: Michael Collado <collado.mike@gmail.com>
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.

2 participants