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

Schedule dispatch frame #1012

Conversation

splhack
Copy link
Contributor

@splhack splhack commented Aug 6, 2021

Problem

As I explained in #1001, findNextDispatchFrames returns the exact same frames in the exact same order when multiple threads or multiple Cuebot machines calls it at the same time. Only one caller of findNextDispatchFrames will succeed to dispatch the frames, the all other threads and machines fail with this error message.

frame reservation error, dispatchProcToJob failed to book next frame, com.imageworks.spcue.dispatcher.FrameReservationException: the frame ... was updated by another thread.

It is wasting CPU, PostgreSQL transaction, and time.

Solution

Introduce b_scheduled boolean and ts_scheduled timestamp to Frame table.
Cuebot updates b_scheduled when it dispatches the frames.
So the other threads can filter out b_scheduled=true frames for their dispatch.

(The original idea, adding a new frame state involves more logic change, database functions, etc. gave up)

Implementation

The original findNextDispatchFrames SQL is SELECT.
This PR will change it to UPDATE for updating b_scheduled and ts_scheduled.
But at the same time it will also append RETURNING clause to fetch the updated frames.
Thus the functionality will remain the same (1 atomic transaction, it will return the exact same frames in the exact same order by the WHERE, ORDER BY, and LIMIT. The exception is that it won't return b_scheduled=true frame), but be capable to update b_scheduled and ts_scheduled.

ts_scheduled is for a fail-safe logic. Cuebot may crash after b_scheduled update and then the frame will be stranded. The solution is that Cuebot will use ts_scheduled timestamp as timeout to reschedule the frame again.

ScheduledDispatchFrames is a convenient class which implements AutoCloseable and Iterable to easily ensure resetting b_scheduled to false (unschedule) when the thread didn't dispatch the scheduled frames.

Summary

  • findNextDispatchFramesscheduledNextDispatchFrames
  • FIND_DISPATCH_FRAME SELECT queries → SCHEDULE_DISPATCH_FRAME UPDATE queries with RETURNING
  • List<DispatchFrame> frames = findNextDispatchFramestry (ScheduledDispatchFrames frames = scheduleNextDispatchFrames)
  • And comprehensive unit tests

@splhack
Copy link
Contributor Author

splhack commented Aug 7, 2021

Update

Problem

Unfortunately spring JdbcTemplate doesn't allow UPDATE in query due to read-only transaction. And I was not able to find a method for UPDATE + RETURNING clause (read-write transaction and querying the values.) (wrong)

Solution

For a workaround of JdbcTemplate, I split the 1 atomic transaction into 2 transactions.

  1. UPDATE to set UUID value to str_scheduled_by.
  2. SELECT to get str_scheduled_by=theUUID frames.

Changes

  • SQL: as mentioned above
  • Migration: change b_schedule boolean to str_scheduled_by string (default NULL)
  • WHEN b_schedule=falseWHEN str_scheduled_by IS NULL
  • Delete dup line in FrameDaoJdbc.java

@bcipriano
Copy link
Collaborator

@DiegoTavares Thoughts?

@DiegoTavares
Copy link
Collaborator

I was convinced in the first proposal. With an atomic operation performance and synchronization issues could be discarded. I did some reading and it looks to be possible to use JdbcTemplate.queryinstead of jdbcTemplate.update to execute a UPDATE + RETURNING clause, have you tried that?

Sorry for the delayed response.

@DiegoTavares
Copy link
Collaborator

Closing up stale PRs.

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.

3 participants