Skip to content

fix cache populate incorrect content when numBackgroundThreads>1 on b…#5694

Closed
kaijianding wants to merge 1 commit intoapache:masterfrom
kaijianding:cachebug
Closed

fix cache populate incorrect content when numBackgroundThreads>1 on b…#5694
kaijianding wants to merge 1 commit intoapache:masterfrom
kaijianding:cachebug

Conversation

@kaijianding
Copy link
Copy Markdown
Contributor

…roker

it's a follow up of #3943. the same bug exists on broker side.

Copy link
Copy Markdown
Contributor

@gianm gianm left a comment

Choose a reason for hiding this comment

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

Is this really a bug? In #3943 the cacheResults.add call was done in a separate thread, meaning they could be added out of order. And the fix was to instead create a list of futures, not a list of results, and add the futures in the proper order.

But in the code being changed in this patch, we already had a list of futures, and it was already in the proper order. So it doesn't look like a bug. Right?

@kaijianding
Copy link
Copy Markdown
Contributor Author

my mis-understanding. it is different with what happens in #3943 now, close this PR.

@kaijianding kaijianding deleted the cachebug branch March 25, 2023 19:09
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