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

Retry to initialize the token store correctly on exception for PSEP. #2277

Merged
merged 2 commits into from Jul 12, 2022

Conversation

gklijs
Copy link
Contributor

@gklijs gklijs commented Jul 7, 2022

This is a fix for #2274

If the token store wasn't initialized the first time, this will keep trying to initialize the token store.

Current state will try every 100 ms from the coordinator to initialize the tokens, until 30 tries, so about 3 seconds. if still failing it throws an exception.

@gklijs gklijs added Type: Bug Use to signal issues that describe a bug within the system. Priority 1: Must Highest priority. A release cannot be made if this issue isn’t resolved. Status: In Progress Use to signal this issue is actively worked on. labels Jul 7, 2022
@gklijs gklijs added this to the Release 4.5.13 milestone Jul 7, 2022
@gklijs gklijs requested a review from smcvb July 7, 2022 11:45
@gklijs gklijs self-assigned this Jul 7, 2022
@CLAassistant
Copy link

CLAassistant commented Jul 7, 2022

CLA assistant check
All committers have signed the CLA.

@CodeDrivenMitch
Copy link
Member

I feel this is not the right way to fix this problem. Looking at the TrackingEventProcessor, the initialisation of the token store happens in a Thread started on a lifecycle handler. However, for the PooledStreamingEventProcessor, it's done on the same Thread as the Spring boot intialisation, blocking further configuration from initializing.

I would suggest:

  • Moving the initialization to a thread
  • Add an incremental backoff
  • Use a while loop to prevent stack overflow

@gklijs gklijs marked this pull request as draft July 7, 2022 13:35
@gklijs gklijs marked this pull request as ready for review July 7, 2022 16:06
@gklijs gklijs force-pushed the fix_issue_2274 branch 2 times, most recently from d77e0e3 to 72a1c63 Compare July 8, 2022 08:04
Copy link
Member

@smcvb smcvb left a comment

Choose a reason for hiding this comment

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

Bunch of things to discuss.

@gklijs gklijs force-pushed the fix_issue_2274 branch 3 times, most recently from 18d338b to 7fdefa7 Compare July 8, 2022 14:06
Copy link
Member

@smcvb smcvb left a comment

Choose a reason for hiding this comment

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

Getting there! Couple of arguments again, though.

Copy link
Member

@smcvb smcvb left a comment

Choose a reason for hiding this comment

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

Being nitpicky here, mostly. But I assume we can resolve these, so I am preemptively approving.

@sonarcloud
Copy link

sonarcloud bot commented Jul 12, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

89.4% 89.4% Coverage
0.0% 0.0% Duplication

@gklijs gklijs merged commit 7dc550f into axon-4.5.x Jul 12, 2022
@gklijs gklijs deleted the fix_issue_2274 branch July 12, 2022 14:36
@gklijs gklijs removed the Status: In Progress Use to signal this issue is actively worked on. label Jul 12, 2022
@gklijs gklijs added the Status: Resolved Use to signal that work on this issue is done. label Jul 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority 1: Must Highest priority. A release cannot be made if this issue isn’t resolved. Status: Resolved Use to signal that work on this issue is done. Type: Bug Use to signal issues that describe a bug within the system.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants