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

[RHPAM-4872] Fix RepeatMode=FIXED when deleted records > RecordsPerTransaction #2374

Merged
merged 1 commit into from
Jan 22, 2024

Conversation

martinweiler
Copy link
Contributor

This is following up on the work from #2292

The problem with job rescheduling still occurs when 'pagination' exists (There are more records to delete than the 'recordsPerTransaction' parameter). When this happens, in each page it ends up moving the date and time of next execution by not taking into account the date of the first page of the current cycle but using the current one.

This PR addresses the issue by storing the execution time of the original LogCleanupCommand.

Copy link

sonarcloud bot commented Jan 5, 2024

@fjtirado fjtirado added backport-7.67.x Generate backport PR for 7.67.x branch backport-7.67.x-blue Generate backport PR for 7.67.x-blue branch labels Jan 8, 2024
@mareknovotny
Copy link
Member

jenkins do fdb

Copy link
Member

@gmunozfe gmunozfe 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 to me, good work @martinweiler
Just a minor comment related to the test timeout in case the test execution is close to 10 seconds, it could be greater.

Comment on lines +268 to +270
// time difference between first and last should be around 10 seconds, even if the original command got split into two executions due to RecordsPerTransaction
long diff = lastExecution - firstExecution;
assertTrue(diff < 11000);
Copy link
Member

Choose a reason for hiding this comment

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

Test timeout is set to 10 seconds, so probably it could be greater in case to let this comparison as it is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gmunozfe Thanks for the input. However, the test is not waiting for the NextRun=10s to actually getting executed, test execution should be done in ~4s. The reason I chose 10s for the NextRun parameter is to make the assert clearer.

@mareknovotny
Copy link
Member

Failed tests not related to changes:

  • org.kie.server.integrationtests.jbpm.jms/JmsResponseHandlerIntegrationTest/testStartAndCompleteTaskUseOfAsyncResponseHandler_JSON_
    Synchronization failed for defined timeout: 240000 milliseconds.
  • org.kie.bc.DeploymentIT.org.kie.bc.DeploymentIT - this is random flaky test

@gmunozfe
Copy link
Member

@mareknovotny this is ready to merge

@mareknovotny mareknovotny merged commit 56f1e04 into kiegroup:main Jan 22, 2024
6 of 9 checks passed
github-actions bot pushed a commit that referenced this pull request Jan 22, 2024
github-actions bot pushed a commit that referenced this pull request Jan 22, 2024
mareknovotny pushed a commit that referenced this pull request Jan 23, 2024
…ansaction (#2374) (#2386)

Co-authored-by: Martin Weiler <mweiler@redhat.com>
mareknovotny pushed a commit that referenced this pull request Jan 23, 2024
…ansaction (#2374) (#2387)

Co-authored-by: Martin Weiler <mweiler@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-7.67.x Generate backport PR for 7.67.x branch backport-7.67.x-blue Generate backport PR for 7.67.x-blue branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants