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

Use pg_advisory_unlock_all after each thread's job execution; fix Lockable return values; improve test stability #285

Merged
merged 1 commit into from
Jun 29, 2021

Conversation

bensheldon
Copy link
Owner

@bensheldon bensheldon commented Jun 25, 2021

When under heavy load, as seen in tests, for an indeterminate reason, job records remain advisory locked. Rather than unlocking each record individually, this unlocks all advisory locks for the thread's database session.

Other functional improvements:

  • Fixes locking query functions that return void
  • Fixes return values of boolean postgres functions; using exists? would always return true in Ruby even if the function (e.g. pg_advisory_unlock had a false value in the query)

Test Improvements:

  • Stubs new TestJob constant rather than stubbing over existing ExampleJob because JRuby sometimes does not stub properly and it becomes confusing to debug
  • Pulls up Postgres Notice messages in tests that otherwise would go to stderr, e.g. "WARNING: you don't own a lock of type ExclusiveLock"
  • Fix Appraisal gem cache
  • A lot of thread-based connection and advisory lock logging, that could be removed in the future.

@bensheldon bensheldon force-pushed the jruby-tests branch 29 times, most recently from 7db9415 to b4d9686 Compare June 28, 2021 16:09
@bensheldon bensheldon force-pushed the jruby-tests branch 4 times, most recently from 720d17d to 5034aac Compare June 28, 2021 22:23
@bensheldon bensheldon changed the title Ensure executors in tests are shutdown after each example Use pg_advisory_unlock_all after each thread's job execution Jun 29, 2021
@bensheldon bensheldon changed the title Use pg_advisory_unlock_all after each thread's job execution Use pg_advisory_unlock_all after each thread's job execution and enhanced test stability Jun 29, 2021
@bensheldon bensheldon force-pushed the jruby-tests branch 3 times, most recently from be37544 to ae7c3fe Compare June 29, 2021 14:17
@bensheldon bensheldon changed the title Use pg_advisory_unlock_all after each thread's job execution and enhanced test stability Use pg_advisory_unlock_all after each thread's job execution; improve test stability Jun 29, 2021
@bensheldon bensheldon changed the title Use pg_advisory_unlock_all after each thread's job execution; improve test stability Use pg_advisory_unlock_all after each thread's job execution; fix Lockable return values; improve test stability Jun 29, 2021
@bensheldon bensheldon added the bug Something isn't working label Jun 29, 2021
@bensheldon bensheldon merged commit 4a26770 into main Jun 29, 2021
@bensheldon bensheldon deleted the jruby-tests branch June 29, 2021 16:12
@bensheldon bensheldon added the enhancement New feature or request label Jul 25, 2021
@bensheldon bensheldon restored the jruby-tests branch October 16, 2023 14:27
@bensheldon
Copy link
Owner Author

I think I discovered the root cause of why this change was introduced because of there being unlocked rows being returned. This is the query being used at the time of this PR:

SELECT "good_jobs".* 
FROM "good_jobs" 
WHERE "good_jobs"."id" IN (
  WITH "rows" AS MATERIALIZED ( 
    SELECT "good_jobs"."id" 
    FROM "good_jobs" 
    WHERE "good_jobs"."finished_at" IS NULL 
    AND ("good_jobs"."scheduled_at" <= '2023-10-16 14:31:18.254122' OR "good_jobs"."scheduled_at" IS NULL) 
    ORDER BY priority DESC NULLS LAST 
  ) 
  SELECT "rows"."id" 
  FROM "rows" 
  WHERE pg_try_advisory_lock(('x' || substr(md5('good_jobs' || "rows"."id"::text), 1, 16))::bit(64)::bigint) 
  LIMIT $1 
  FOR UPDATE SKIP LOCKED --- <= The problematic addition 🚨
) ORDER BY priority DESC NULLS LAST

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant