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

server: process wait conditions in batch mode #910

Merged
merged 4 commits into from
May 23, 2024

Conversation

brig
Copy link
Contributor

@brig brig commented May 16, 2024

the idea here is this: instead of processing each wait condition individually, we will process them in batches. As a result, instead of requesting the status for each process separately, we will request the statuses for a batch of processes at once

@brig brig added the wip Work in progress, do not merge label May 16, 2024
@brig brig force-pushed the brig/wait-conditions-batch branch 2 times, most recently from 6999b84 to b3d8c73 Compare May 20, 2024 04:24
@brig brig changed the title [WIP] server: process wait conditions in batch mode server: process wait conditions in batch mode May 20, 2024
@brig brig force-pushed the brig/wait-conditions-batch branch from b3d8c73 to 8a2282d Compare May 20, 2024 16:12
Copy link
Collaborator

@ibodrov ibodrov left a comment

Choose a reason for hiding this comment

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

Still looking through it, one Q so far.

ProcessQueue q = PROCESS_QUEUE.as("q");
return tx.select(q.INSTANCE_ID, q.CURRENT_STATUS)
.from(q)
.where(q.INSTANCE_ID.in(processes))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is JOOQ smart enough to turn this INSTANCE_ID IN (...) into an array operation? I'm worried that it's going to fail for large processes sets.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

So the upper limit is waitProcessLimitForStatusQuery? Should be fine, I think, but for the query parser sake I'd replaced with an array operation if it is possible here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, waitProcessLimitForStatusQuery is a limit

@brig brig removed the wip Work in progress, do not merge label May 20, 2024
@brig brig marked this pull request as ready for review May 20, 2024 22:12
@ibodrov ibodrov merged commit e73c078 into master May 23, 2024
2 checks passed
@ibodrov ibodrov deleted the brig/wait-conditions-batch branch May 23, 2024 22:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants