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

FINERACT-2067: NPE business date partition step COB #4056

Conversation

kulminsky
Copy link
Contributor

@kulminsky kulminsky commented Sep 13, 2024

Description

This is to fix an NPE issue we are seeing in some of our test environments during COB in multi-tenant mode.

Possible cause:

  • When running in multi-tenant mode, ES and IT COBs execute at the same time.
  • Both of these could somehow both be trying to access the business date value, causing NPE intermittently.

Ignore if these details are present on the associated Apache Fineract JIRA ticket.

Checklist

Please make sure these boxes are checked before submitting your pull request - thanks!

  • Write the commit message as per https://github.com/apache/fineract/#pull-requests

  • Acknowledge that we will not review PRs that are not passing the build ("green") - it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers.

  • Create/update unit or integration tests for verifying the changes made.

  • Follow coding conventions at https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions.

  • Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/legacy-docs/apiLive.htm with details of any API changes

  • Submission is not a "code dump". (Large changes can be made "in repository" via a branch. Ask on the developer mailing list for guidance, if required.)

FYI our guidelines for code reviews are at https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide.

@kulminsky kulminsky changed the title FINERACT-2067 NPE_business_date_partition_step_COB FINERACT-2067 NPE business date partition step COB Sep 13, 2024
@galovics
Copy link
Contributor

@kulminsky can you pls explain what was the exact issue you solved? I'm not sure I understand the purpose of the code changes yet. Thanks.

@adamsaghy
Copy link
Contributor

@kulminsky Please make sure to follow the following format for PR title and commit message:
FINERACT-XXXX: <short message>

The ":" was missing from your actual title and commit message, but the above one is used to automatically pick up during a release and connect with the relevant jira ticket.

@kulminsky
Copy link
Contributor Author

@kulminsky Please make sure to follow the following format for PR title and commit message: FINERACT-XXXX: <short message>

The ":" was missing from your actual title and commit message, but the above one is used to automatically pick up during a release and connect with the relevant jira ticket.

Sorry, I saw the comment only after the commit. I will follow the rules in future commits.

@kulminsky kulminsky changed the title FINERACT-2067 NPE business date partition step COB FINERACT-2067: NPE business date partition step COB Sep 13, 2024
@adamsaghy adamsaghy marked this pull request as draft September 13, 2024 15:30
@kulminsky
Copy link
Contributor Author

@kulminsky can you pls explain what was the exact issue you solved? I'm not sure I understand the purpose of the code changes yet. Thanks.

@galovics
In general, this is to fix an NPE issue we are seeing in some of our test environments in multi-tenant mode.
Potential fix steps:

  1. Make partition step StepScoped.

  2. This creates a new instance of the partition step, instead of the default singleton in Spring batch.

  3. Copy values from job execution context into step execution context before partition step.

  4. We then give the business date value to the step context before the step runs, ensuring other jobs/steps are not trying to access the business date value.

@kulminsky kulminsky force-pushed the FINERACT-2067-NPE_business_date_partition_step_COB branch from 43689bd to 9531ff3 Compare September 13, 2024 16:13
@kulminsky kulminsky force-pushed the FINERACT-2067-NPE_business_date_partition_step_COB branch from 9531ff3 to b6bd067 Compare September 17, 2024 11:29
@kulminsky kulminsky marked this pull request as ready for review September 17, 2024 11:30
@kulminsky kulminsky force-pushed the FINERACT-2067-NPE_business_date_partition_step_COB branch from b6bd067 to 6fa5a6d Compare September 17, 2024 13:26
Copy link
Contributor

@adamsaghy adamsaghy left a comment

Choose a reason for hiding this comment

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

LGTM

@adamsaghy adamsaghy merged commit 7b058d6 into apache:develop Sep 18, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants