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

Upgrade to AspectJ 1.9.20.1 #407

Merged
merged 1 commit into from
Dec 12, 2023
Merged

Conversation

staniakm
Copy link
Contributor

No description provided.

Copy link
Member

@artembilan artembilan left a comment

Choose a reason for hiding this comment

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

Thanks for the update, but any chances that you can add a respective unit test which fails without that version bumping?

@staniakm
Copy link
Contributor Author

Thanks for the update, but any chances that you can add a respective unit test which fails without that version bumping?

I'm looking for a way to unit test it but I don't see any tests that check if retry mechanism waits for some period of time before retry.

@artembilan
Copy link
Member

Well, you showed one in the issue: #406.
Then I see that we have EnableRetryTests.ExpressionService which provides a config similar to what you show in the issue.
Perhaps that's from where you can start unit testing it.

Would be great to see how it fails for you, so then we might be able to even just adjust existing test (and its configuration) to fail.
If it is only about Kotlin, then yeah, we don't have choice unless just accept the fix as is.

@staniakm
Copy link
Contributor Author

Nah, its also fails with java. Made the same test with spring java service.

Yeah, I have found this test EnableRetryTests.ExpressionService. Not yet sure how to calcualte elapsed time because now method execution finish really quick without consider delays defined by backoff.

@artembilan
Copy link
Member

I'm not sure how is the timing is relevant to the failure you are claiming with AspectJ.

Any explanations in behavior difference before and after upgrade?

@staniakm
Copy link
Contributor Author

I don't really know why aspectJ affect retry delay. But with version 2.0.* first retry wait delayExpresion time, but next one is not using multipliedExpression to calculate delay and still using delayExpresion. So, for example with delayExpression="500" and multiplier "2.0", delay between method execution will be 500 every time. With old verions 1.3+ or with upadted aspectJ, delay will use multiplier, so delays between next method executions will be 500, 1000, 2000...

I have found solution how to calculate time in unit tests using DummySleeper, but it looks like unit tests are not affected by aspectJ issu

@staniakm
Copy link
Contributor Author

@artembilan
Copy link
Member

artembilan commented Dec 12, 2023

Well, this one:

	@Retryable(backoff = @Backoff(delayExpression = "500", multiplierExpression = "2.0"),
			maxAttempts = 4)
	public void toBeRetried() {
		throw new IllegalArgumentException("some exception");
	}

fails for me even with AspectJ 1.9.20.1:

2023-12-12T15:08:13.985-05:00 DEBUG 22384 --- [    Test worker] o.s.retry.support.RetryTemplate          : Retry: count=0
2023-12-12T15:08:13.987-05:00 DEBUG 22384 --- [    Test worker] o.s.r.backoff.ExponentialBackOffPolicy   : Sleeping for 500
2023-12-12T15:08:14.502-05:00 DEBUG 22384 --- [    Test worker] o.s.retry.support.RetryTemplate          : Checking for rethrow: count=1
2023-12-12T15:08:14.502-05:00 DEBUG 22384 --- [    Test worker] o.s.retry.support.RetryTemplate          : Retry: count=1
2023-12-12T15:08:14.502-05:00 DEBUG 22384 --- [    Test worker] o.s.r.backoff.ExponentialBackOffPolicy   : Sleeping for 500
2023-12-12T15:08:15.011-05:00 DEBUG 22384 --- [    Test worker] o.s.retry.support.RetryTemplate          : Checking for rethrow: count=2
2023-12-12T15:08:15.011-05:00 DEBUG 22384 --- [    Test worker] o.s.retry.support.RetryTemplate          : Retry: count=2
2023-12-12T15:08:15.011-05:00 DEBUG 22384 --- [    Test worker] o.s.r.backoff.ExponentialBackOffPolicy   : Sleeping for 500
2023-12-12T15:08:15.512-05:00 DEBUG 22384 --- [    Test worker] o.s.retry.support.RetryTemplate          : Checking for rethrow: count=3
2023-12-12T15:08:15.512-05:00 DEBUG 22384 --- [    Test worker] o.s.retry.support.RetryTemplate          : Retry: count=3
2023-12-12T15:08:15.512-05:00 DEBUG 22384 --- [    Test worker] o.s.retry.support.RetryTemplate          : Checking for rethrow: count=4
2023-12-12T15:08:15.513-05:00 DEBUG 22384 --- [    Test worker] o.s.retry.support.RetryTemplate          : Retry failed last attempt: count=4

but if I do this:

  implementation 'org.springframework.retry:spring-retry:2.0.5-SNAPSHOT'

Then all is good:

2023-12-12T15:09:11.946-05:00 DEBUG 1208 --- [    Test worker] o.s.retry.support.RetryTemplate          : Retry: count=0
2023-12-12T15:09:11.949-05:00 DEBUG 1208 --- [    Test worker] o.s.r.backoff.ExponentialBackOffPolicy   : Sleeping for 500
2023-12-12T15:09:12.459-05:00 DEBUG 1208 --- [    Test worker] o.s.retry.support.RetryTemplate          : Checking for rethrow: count=1
2023-12-12T15:09:12.459-05:00 DEBUG 1208 --- [    Test worker] o.s.retry.support.RetryTemplate          : Retry: count=1
2023-12-12T15:09:12.460-05:00 DEBUG 1208 --- [    Test worker] o.s.r.backoff.ExponentialBackOffPolicy   : Sleeping for 1000
2023-12-12T15:09:13.465-05:00 DEBUG 1208 --- [    Test worker] o.s.retry.support.RetryTemplate          : Checking for rethrow: count=2
2023-12-12T15:09:13.465-05:00 DEBUG 1208 --- [    Test worker] o.s.retry.support.RetryTemplate          : Retry: count=2
2023-12-12T15:09:13.465-05:00 DEBUG 1208 --- [    Test worker] o.s.r.backoff.ExponentialBackOffPolicy   : Sleeping for 2000
2023-12-12T15:09:15.469-05:00 DEBUG 1208 --- [    Test worker] o.s.retry.support.RetryTemplate          : Checking for rethrow: count=3
2023-12-12T15:09:15.469-05:00 DEBUG 1208 --- [    Test worker] o.s.retry.support.RetryTemplate          : Retry: count=3
2023-12-12T15:09:15.469-05:00 DEBUG 1208 --- [    Test worker] o.s.retry.support.RetryTemplate          : Checking for rethrow: count=4
2023-12-12T15:09:15.469-05:00 DEBUG 1208 --- [    Test worker] o.s.retry.support.RetryTemplate          : Retry failed last attempt: count=4

Apparently you are facing the problem fixed in this issue: #397.

So, I guess we can accept your PR as just a dependency upgrade and that's it.
Just because the problem has nothing to do with AspectJ and the real fix for delayExpression is already in.

We are planning to release 2.0.5 this Friday.

@artembilan artembilan merged commit fb07f61 into spring-projects:main Dec 12, 2023
1 check passed
@artembilan
Copy link
Member

@staniakm ,

thank you for contribution; looking forward for more!

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.

2 participants