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

fix(transaction): ensure transactions occur in correct context/thread #396

Merged
merged 1 commit into from
Apr 24, 2024
Merged

fix(transaction): ensure transactions occur in correct context/thread #396

merged 1 commit into from
Apr 24, 2024

Conversation

andrewazores
Copy link
Member

@andrewazores andrewazores commented Apr 19, 2024 •
edited
Loading

Welcome to Cryostat3! 👋

Before contributing, make sure you have:

  • Read the contributing guidelines
  • Linked a relevant issue which this PR resolves
  • Linked any other relevant issues, PR's, or documentation, if any
  • Resolved all conflicts, if any
  • Rebased your branch PR on top of the latest upstream main branch
  • Attached at least one of the following labels to the PR: [chore, ci, docs, feat, fix, test]
  • Signed all commits using a GPG signature

To recreate commits with GPG signature git fetch upstream && git rebase --force --gpg-sign upstream/main

Fixes: #389
Based on #342

Description of the change:

  • ensures Blocking and Transactional annotations are applied correctly in GraphQL API entrypoints and HTTP API entrypoints
  • disables GraphQL nonblocking support, improves performance but is more difficult to manage the concurrency - not worth gaining performance at the expense of correctness or reliability
  • refactors some code using to ensure that database transactions occur on the calling thread where the transaction context is active, not on some delegated worker

Motivation for the change:

This change is helpful because users may want to...

How to manually test:

  1. Run CRYOSTAT_IMAGE=quay.io... bash smoketest.bash...
  2. ...

@github-actions github-actions bot added dependent needs-triage Needs thorough attention from code reviewers labels Apr 19, 2024
@andrewazores andrewazores removed the needs-triage Needs thorough attention from code reviewers label Apr 19, 2024
@andrewazores andrewazores force-pushed the transaction-ctx-graphql-parallel branch 3 times, most recently from a4113a6 to efb6679 Compare April 23, 2024 18:40
@andrewazores andrewazores marked this pull request as ready for review April 23, 2024 18:40
@andrewazores
Copy link
Member Author

/build_test

@github-actions GitHub Actions
Copy link

This PR/issue depends on:

@andrewazores andrewazores requested a review from mwangggg April 23, 2024 18:41
@github-actions GitHub Actions
Copy link

Workflow started at 4/23/2024, 2:41:20 PM. View Actions Run.

@github-actions GitHub Actions
Copy link

No OpenAPI schema changes detected.

@github-actions GitHub Actions
Copy link

No GraphQL schema changes detected.

@github-actions GitHub Actions
Copy link

CI build and push: All tests pass ✅ (JDK17)
https://github.com/cryostatio/cryostat3/actions/runs/8805568298

@andrewazores andrewazores force-pushed the transaction-ctx-graphql-parallel branch from efb6679 to aff1c63 Compare April 23, 2024 19:07
mwangggg
mwangggg approved these changes Apr 24, 2024
View reviewed changes
Copy link
Member

@mwangggg mwangggg left a comment

Choose a reason for hiding this comment

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

looks good, just needs a rebase

@mwangggg
Copy link
Member

/build_test

@github-actions GitHub Actions
Copy link

Workflow started at 4/24/2024, 1:21:14 PM. View Actions Run.

@github-actions GitHub Actions
Copy link

No GraphQL schema changes detected.

@github-actions GitHub Actions
Copy link

No OpenAPI schema changes detected.

@andrewazores andrewazores force-pushed the transaction-ctx-graphql-parallel branch from aff1c63 to 629a0f0 Compare April 24, 2024 17:24
@github-actions GitHub Actions
Copy link

CI build and push: All tests pass ✅ (JDK17)
https://github.com/cryostatio/cryostat3/actions/runs/8820650238

@andrewazores
Copy link
Member Author

/build_test

@github-actions GitHub Actions
Copy link

Workflow started at 4/24/2024, 1:33:09 PM. View Actions Run.

@github-actions GitHub Actions
Copy link

No OpenAPI schema changes detected.

@github-actions GitHub Actions
Copy link

No GraphQL schema changes detected.

@github-actions GitHub Actions
Copy link

CI build and push: All tests pass ✅ (JDK17)
https://github.com/cryostatio/cryostat3/actions/runs/8820809828

@andrewazores andrewazores merged commit aa5ab4e into cryostatio:main Apr 24, 2024
8 checks passed
@andrewazores andrewazores deleted the transaction-ctx-graphql-parallel branch April 24, 2024 17:40
@andrewazores andrewazores mentioned this pull request Apr 24, 2024
Merged
7 tasks
@andrewazores andrewazores mentioned this pull request Sep 3, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mwangggg mwangggg mwangggg approved these changes

Assignees
No one assigned
Labels
fix safe-to-test
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug] Topology view actions sometimes silently partially fail
2 participants
@andrewazores @mwangggg