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.

Sign up for GitHub

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

Jump to bottom

feat: avoid concurrent writes to jena #1577

Merged
merged 23 commits into from
Jul 12, 2023
Merged

feat: avoid concurrent writes to jena #1577

merged 23 commits into from
Jul 12, 2023

Conversation

eikek
Copy link
Member

@eikek eikek commented Jun 27, 2023 •
edited
Loading

This PR makes the event handlers writing to TS sequentially to avoid concurrent writes that lead to duplicates.

/deploy #persist extra-values=core.sentry.dsn=https://0cc02f3a02d3474d9f7e90915873ab4d@sentry.dev.renku.ch/2,core.sentry.environment=renku-dev,core.sentry.enabled=true

@jachro
Copy link
Contributor

jachro commented Jun 29, 2023 •
edited
Loading

This looks really cool! Cannot wait to see it in action :)

I couldn't approve it as it's still in draft but feel free to merge it whenever you feel it's ready.

@eikek eikek marked this pull request as ready for review June 30, 2023 14:09
@eikek eikek requested review from a team as code owners June 30, 2023 14:09
@eikek eikek temporarily deployed to renku-ci-gr-1577 June 30, 2023 14:15 — with GitHub Actions Inactive
@RenkuBot
Copy link
Collaborator

You can access the deployment of this PR at https://renku-ci-gr-1577.dev.renku.ch

@eikek eikek temporarily deployed to renku-ci-gr-1577 July 4, 2023 09:06 — with GitHub Actions Inactive
@eikek eikek temporarily deployed to renku-ci-gr-1577 July 4, 2023 14:40 — with GitHub Actions Inactive
@eikek eikek temporarily deployed to renku-ci-gr-1577 July 5, 2023 08:42 — with GitHub Actions Inactive
@eikek eikek temporarily deployed to renku-ci-gr-1577 July 5, 2023 08:45 — with GitHub Actions Inactive
@eikek eikek temporarily deployed to renku-ci-gr-1577 July 5, 2023 11:14 — with GitHub Actions Inactive
@eikek eikek temporarily deployed to renku-ci-gr-1577 July 6, 2023 06:37 — with GitHub Actions Inactive
@eikek eikek temporarily deployed to renku-ci-gr-1577 July 11, 2023 14:38 — with GitHub Actions Inactive
@eikek eikek temporarily deployed to renku-ci-gr-1577 July 11, 2023 16:31 — with GitHub Actions Inactive
jachro
jachro reviewed Jul 11, 2023
View reviewed changes
acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/flows/TSProvisioning.scala Outdated
@@ -74,7 +75,9 @@ trait TSProvisioning
sleep((5 seconds).toMillis)
}

`wait for events to be processed`(project.id, accessToken, commitIds.size)
val items = eventLogClient.getEvents(project.id.asLeft).unsafeRunSync().size
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, my idea was slightly different. What I imagined was to pass the expected number of events to the wait for events to be processed and only there call the new eventLogClient.getEvents and check if the expected number equals the size from the getEvents.

Copy link
Member Author

Choose a reason for hiding this comment

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

But why would we want to first "guess" and then check? I thought this is more of a helper method to only wait for things so the tests can continue, not really part of the test?

@eikek eikek temporarily deployed to renku-ci-gr-1577 July 12, 2023 05:41 — with GitHub Actions Inactive
@eikek eikek temporarily deployed to renku-ci-gr-1577 July 12, 2023 06:22 — with GitHub Actions Inactive
@eikek eikek temporarily deployed to renku-ci-gr-1577 July 12, 2023 07:08 — with GitHub Actions Inactive
@eikek eikek temporarily deployed to renku-ci-gr-1577 July 12, 2023 07:59 — with GitHub Actions Inactive
@eikek eikek temporarily deployed to renku-ci-gr-1577 July 12, 2023 09:14 — with GitHub Actions Inactive
@eikek eikek temporarily deployed to renku-ci-gr-1577 July 12, 2023 11:12 — with GitHub Actions Inactive
@eikek eikek temporarily deployed to renku-ci-gr-1577 July 12, 2023 12:18 — with GitHub Actions Inactive
@eikek eikek deployed to renku-ci-gr-1577 July 12, 2023 12:21 — with GitHub Actions Active
@jachro jachro linked an issue Jul 12, 2023 that may be closed by this pull request
Create a service to solve the problem of concurrent TS updates #1282
Closed
7 tasks
@eikek eikek merged commit 15255b2 into development Jul 12, 2023
12 checks passed
@eikek eikek deleted the 1282-ts-update-lock branch July 12, 2023 13:08
@jachro jachro mentioned this pull request Jul 25, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jachro jachro jachro left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Create a service to solve the problem of concurrent TS updates
3 participants
@eikek @jachro @RenkuBot