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

feat : handle thread safe issue for getCurrentFutures #1843

Merged

Conversation

younseunghyun
Copy link
Contributor

📝 Description

improve thread safe issue for getCurrentFutures

current implementation get the values form cachemap for dataloaders.

And make it flatten. But when we get the values it's just iterator, not all the elements is obtained.

So when we run flatten, it traverse the each iterator, and it make ArrayIndexOutOfBoundException

Following is error message

aused by: java.lang.ArrayIndexOutOfBoundsException: Index 279 out of bounds for length 279
	at java.base/java.util.HashMap.valuesToArray(HashMap.java:973)
	at java.base/java.util.HashMap$Values.toArray(HashMap.java:1050)
	at java.base/java.util.ArrayList.addAll(ArrayList.java:670)
	at kotlin.collections.CollectionsKt__MutableCollectionsKt.addAll(MutableCollections.kt:116)
	at kotlin.collections.CollectionsKt__IterablesKt.flatten(Iterables.kt:49)
	at com.expediagroup.graphql.dataloader.KotlinDataLoaderRegistry.getCurrentFutures(KotlinDataLoaderRegistry.kt:67)
	at com.expediagroup.graphql.dataloader.KotlinDataLoaderRegistry.dispatchAll(KotlinDataLoaderRegistry.kt:78)
	at graphql.execution.instrumentation.dataloader.FieldLevelTrackingApproach.dispatch(FieldLevelTrackingApproach.java:242)
	at graphql.execution.instrumentation.dataloader.FieldLevelTrackingApproach$1.onFieldValuesInfo(FieldLevelTrackingApproach.java:150)
	at graphql.execution.AsyncExecutionStrategy.lambda$execute$1(AsyncExecutionStrategy.java:72)
	at java.base/java.util.concurrent.CompletableFuture.uniWhenComplete(CompletableFuture.java:863)
	... 50 common frames omitted

🔗 Related Issues

#1837

@younseunghyun
Copy link
Contributor Author

@samuelAndalon

@younseunghyun
Copy link
Contributor Author

there was ktilntFormatting Issue
I refac code and overwrite commit

@samuelAndalon
Copy link
Contributor

Afk right now, once merged can you cherry pick to 6.x.x ?

@younseunghyun
Copy link
Contributor Author

@samuelAndalon
got it.

@younseunghyun
Copy link
Contributor Author

@samuelAndalon cherry pick to 6.x.x branch

samuelAndalon pushed a commit that referenced this pull request Sep 14, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
cherry pick #1843
@dariuszkuc dariuszkuc added type: bug Something isn't working changes: patch Changes require a patch version labels Sep 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changes: patch Changes require a patch version type: bug Something isn't working
Development

Successfully merging this pull request may close these issues.

None yet

3 participants