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

CORE-19295: Clean Up Checkpoints Only #5439

Merged
merged 3 commits into from
Jan 16, 2024
Merged

CORE-19295: Clean Up Checkpoints Only #5439

merged 3 commits into from
Jan 16, 2024

Conversation

jujoramos
Copy link
Contributor

Fix "FlowTimeoutTaskProcessor" implementation so that only states
representing flow checkpoints are marked for deletion. Other types of
states should not be automatically deleted as that might cause issues,
especially when the persistent storage is shared to store multiple
state types.

@corda-jenkins-ci02
Copy link
Contributor

corda-jenkins-ci02 bot commented Jan 15, 2024 •
edited
Loading

Jenkins build for PR 5439 build 4

Build Successful:
Jar artifact version produced by this PR: 5.2.0.0-alpha-1705337458572
Helm chart version produced by this PR: 5.2.0-alpha.1705337458572
Helm chart pushed to: oci://corda-os-docker-dev.software.r3.com/helm-charts/pr-5439/corda

@jujoramos
CORE-19295: Clean Up Checkpoints Only
9f727fd 
Fix "FlowTimeoutTaskProcessor" implementation so that only states
representing flow checkpoints are marked for deletion. Other types of
states should not be automatically deleted as that might cause issues,
especially when the persistent storage is shared  to store multiple
state types.
@jujoramos
Trigger CI
30ece9d
@jujoramos
Trigger CI
51f0392
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@jujoramos jujoramos marked this pull request as ready for review January 15, 2024 17:14
@jujoramos jujoramos requested review from a team as code owners January 15, 2024 17:14
anien
anien approved these changes Jan 15, 2024
View reviewed changes
Copy link
Contributor

@anien anien left a comment

Choose a reason for hiding this comment

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

LGTM (only crypto).

JamesHR3
JamesHR3 approved these changes Jan 16, 2024
View reviewed changes
Copy link
Collaborator

@JamesHR3 JamesHR3 left a comment

Choose a reason for hiding this comment

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

This is safer in the existing code, but will prevent cleanup where the processor ends up not getting a say (e.g. timeout of processing an event that has not yet generated a state).

LWogan
LWogan approved these changes Jan 16, 2024
View reviewed changes
Copy link
Contributor

@LWogan LWogan left a comment

Choose a reason for hiding this comment

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

lgtm

@JamesHR3 JamesHR3 merged commit 4955c5f into corda:release/os/5.2 Jan 16, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LWogan LWogan LWogan approved these changes

@JamesHR3 JamesHR3 JamesHR3 approved these changes

@filipesoliveira filipesoliveira filipesoliveira approved these changes

@anien anien anien approved these changes

@conalsmith-r3 conalsmith-r3 Awaiting requested review from conalsmith-r3 conalsmith-r3 is a code owner

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

Successfully merging this pull request may close these issues.
5 participants
@jujoramos @anien @filipesoliveira @JamesHR3 @LWogan