You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@bookkeeper.apache.org by Lari Hotari <lh...@apache.org> on 2022/03/14 08:20:38 UTC

thread safety issue in the BK client, introduced in 4.9.0

Hi all,

There seems to be a severe thread safety issue in the BK client:
"Recycled LedgerEntryImpl instances are corrupted due to a thread safety
issue in BK client"
https://github.com/apache/bookkeeper/issues/3104 .

I assume that we could simply revert PR "Read Submission should bypass OSE
Threads" https://github.com/apache/bookkeeper/pull/1792 . The commit for PR
was part of release 4.9.0 (
https://github.com/apache/bookkeeper/commit/6b99ff73).

This PR which broke the thread safety was made to address a performance
issue, "Read Submission should bypass OSE Threads" ,
https://github.com/apache/bookkeeper/issues/1791.

Since the proposal is to revert the performance optimization, the
alternative mitigation would be to implement
"Improve performance of OrderedExecutor by switching to a more performant
BlockingQueue implementation"
https://github.com/apache/bookkeeper/issues/3105 .

Any feedback on this plan?

There's already a PR https://github.com/apache/bookkeeper/pull/3106 to
revert PR "Read Submission should bypass OSE Threads"
https://github.com/apache/bookkeeper/pull/1792 .
Please review.

BR,

Lari

Re: thread safety issue in the BK client, introduced in 4.9.0

Posted by Enrico Olivelli <eo...@gmail.com>.
Thank you Lari,
great work

I believe it is better to revert that change (I have sent a PR)

We can then do performance tests and see if we need to do something or
if actually performances are not affected so much

Data corruption is a real problem
Better performances is a "nice to have"

Enrico

Il giorno lun 14 mar 2022 alle ore 09:21 Lari Hotari
<lh...@apache.org> ha scritto:
>
> Hi all,
>
> There seems to be a severe thread safety issue in the BK client:
> "Recycled LedgerEntryImpl instances are corrupted due to a thread safety
> issue in BK client"
> https://github.com/apache/bookkeeper/issues/3104 .
>
> I assume that we could simply revert PR "Read Submission should bypass OSE
> Threads" https://github.com/apache/bookkeeper/pull/1792 . The commit for PR
> was part of release 4.9.0 (
> https://github.com/apache/bookkeeper/commit/6b99ff73).
>
> This PR which broke the thread safety was made to address a performance
> issue, "Read Submission should bypass OSE Threads" ,
> https://github.com/apache/bookkeeper/issues/1791.
>
> Since the proposal is to revert the performance optimization, the
> alternative mitigation would be to implement
> "Improve performance of OrderedExecutor by switching to a more performant
> BlockingQueue implementation"
> https://github.com/apache/bookkeeper/issues/3105 .
>
> Any feedback on this plan?
>
> There's already a PR https://github.com/apache/bookkeeper/pull/3106 to
> revert PR "Read Submission should bypass OSE Threads"
> https://github.com/apache/bookkeeper/pull/1792 .
> Please review.
>
> BR,
>
> Lari

Re: thread safety issue in the BK client, introduced in 4.9.0

Posted by Lari Hotari <lh...@apache.org>.
The reported issue
https://github.com/apache/bookkeeper/issues/3104 turned out to be a clear state handling issue in PendingReadOp class (including embedded classes), fix is https://github.com/apache/bookkeeper/pull/3110 .

-Lari

On 2022/03/14 08:20:38 Lari Hotari wrote:
> Hi all,
> 
> There seems to be a severe thread safety issue in the BK client:
> "Recycled LedgerEntryImpl instances are corrupted due to a thread safety
> issue in BK client"
> https://github.com/apache/bookkeeper/issues/3104 .
> 
> I assume that we could simply revert PR "Read Submission should bypass OSE
> Threads" https://github.com/apache/bookkeeper/pull/1792 . The commit for PR
> was part of release 4.9.0 (
> https://github.com/apache/bookkeeper/commit/6b99ff73).
> 
> This PR which broke the thread safety was made to address a performance
> issue, "Read Submission should bypass OSE Threads" ,
> https://github.com/apache/bookkeeper/issues/1791.
> 
> Since the proposal is to revert the performance optimization, the
> alternative mitigation would be to implement
> "Improve performance of OrderedExecutor by switching to a more performant
> BlockingQueue implementation"
> https://github.com/apache/bookkeeper/issues/3105 .
> 
> Any feedback on this plan?
> 
> There's already a PR https://github.com/apache/bookkeeper/pull/3106 to
> revert PR "Read Submission should bypass OSE Threads"
> https://github.com/apache/bookkeeper/pull/1792 .
> Please review.
> 
> BR,
> 
> Lari
>