async-compression icon indicating copy to clipboard operation
async-compression copied to clipboard

Published 20 hours ago • verified publisher icon Nemo157

Fix panic: cannot consume from pending buffer

Open NobodyXu opened this issue 1 year ago • 2 comments

Fixed #298

In this PR I tried a different fix, by making sure buf.consume is always called, even on error. I suspect that previously we didn't consume the buffer on error, and that might have caused the same data to be decompressed again.

I can't think of anywhere else that could fail, the decoder implementation looks alright.

NobodyXu avatar Oct 15 '24 13:10 NobodyXu

@Turbo87 Can you try this PR please?

NobodyXu avatar Oct 15 '24 13:10 NobodyXu

yep, I'll give it a try.

Turbo87 avatar Oct 15 '24 15:10 Turbo87

Bildschirmfoto 2024-10-16 um 09 32 57

unfortunately still failing 😢

Turbo87 avatar Oct 16 '24 09:10 Turbo87

Thanks.

Is the software open-source?

Can I have a look at the code and the test?

NobodyXu avatar Oct 16 '24 11:10 NobodyXu

Can I have a look at the code and the test?

the code yes, the test no. unfortunately we don't have a test that reproduces it. I can only run it on our staging environment where I can reproduce it. our test suite runs with an in-memory object_store instance instead of the S3 implementation we use on staging and production.

Turbo87 avatar Oct 16 '24 12:10 Turbo87

I've found the cause of the panic.

It is because the decoder try to advance the buffer before polling the underlying buf reader.

cc @Turbo87 I've updated the PR, can you try again please?

Thank you!

NobodyXu avatar Oct 16 '24 13:10 NobodyXu

the panic appears to be gone, but we're now seeing an "interval out of range" error result without a stacktrace. I will have to improve our logging a bit to figure out where exactly that is coming from.

Turbo87 avatar Oct 16 '24 16:10 Turbo87

cc @robjtede Shall we merge and publish this for now, since it at least fixes the panic for @Turbo87

NobodyXu avatar Oct 18 '24 21:10 NobodyXu

we're now seeing an "interval out of range" error

it turns out that this was a bug on our side, related to how we calculate exponential backoff for failed jobs.

I can confirm that https://github.com/Nullus157/async-compression/pull/303 appears to fix the issue for us! 🎉

thanks again! :)

Turbo87 avatar Oct 19 '24 08:10 Turbo87

Thank you!

NobodyXu avatar Oct 19 '24 08:10 NobodyXu

Thank you!

NobodyXu avatar Oct 19 '24 08:10 NobodyXu

cc @robjtede let's get this merged and cut a new release, as it is confirmed to fix the panic

NobodyXu avatar Oct 20 '24 02:10 NobodyXu

I will get this merged and ask for review in the release PR.

NobodyXu avatar Oct 20 '24 06:10 NobodyXu

Ahh wonderful, yes, lets get this out today.

robjtede avatar Oct 20 '24 11:10 robjtede

thanks again for the investigation, fix and release! I just merged the latest update into crates.io :)

Turbo87 avatar Oct 20 '24 21:10 Turbo87