Skip to content

Conversation

@rstoyanchev
Copy link
Contributor

@rstoyanchev rstoyanchev commented Feb 18, 2021

I think, on occasion, not all requests had finished executing when the counter is checked.

@rstoyanchev rstoyanchev added this to the 1.1.1 milestone Feb 18, 2021
@rstoyanchev rstoyanchev self-assigned this Feb 18, 2021
@rstoyanchev
Copy link
Contributor Author

It looks like some requests are failing like this:

2021-02-18T11:23:19.6328178Z STD_ERR: java.lang.ClassCastException: class io.rsocket.loadbalance.PooledRSocket cannot be cast to class io.rsocket.loadbalance.WeightedStats (io.rsocket.loadbalance.PooledRSocket and io.rsocket.loadbalance.WeightedStats are in unnamed module of loader 'app')
2021-02-18T11:23:19.6332679Z STD_ERR: 	at io.rsocket.loadbalance.LoadbalanceTest.lambda$shouldDeliverAllTheRequestsWithWeightedStrategy$7(LoadbalanceTest.java:109)
2021-02-18T11:23:19.6336141Z STD_ERR: 	at io.rsocket.loadbalance.WeightedLoadbalanceStrategy.select(WeightedLoadbalanceStrategy.java:72)
2021-02-18T11:23:19.6338775Z STD_ERR: 	at io.rsocket.loadbalance.RSocketPool.doSelect(RSocketPool.java:217)

The test sets up a weightedStatsResolver that simply downcasts the RSocket which is PooledRSocket. So when there are two loadbalance targets, and RSocketPool needs to make a choice, it fails.

@rstoyanchev
Copy link
Contributor Author

I've pushed an update that should address the issue.

@rstoyanchev
Copy link
Contributor Author

I undid the CountDownLatch and force pushed in order to see the actual change that remains. Basically fixing a ClassCastException from the weightedStatsResolver.

Copy link
Member

@OlegDokuka OlegDokuka left a comment

Choose a reason for hiding this comment

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

LGTM

@OlegDokuka OlegDokuka changed the title Fix concurrency issue in LoadbalanceTest fixes LoadbalanceTest issues Feb 19, 2021
@OlegDokuka OlegDokuka merged commit d77df7b into rsocket:master Feb 19, 2021
@rstoyanchev rstoyanchev deleted the LoadTest-issue branch February 19, 2021 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants