[
https://issues.apache.org/jira/browse/HBASE-25984?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17359575#comment-17359575
]
Bharath Vissapragada edited comment on HBASE-25984 at 6/8/21, 9:39 PM:
---
Obtained a heap dump and poked around quite a bit and observed that in-memory
state of the {{RingBufferEventHandler}} is corrupted. After analyzing it and
eyeballing the code (long story short), came up with the following theory...
This is the current implementation of SyncFuture#reset()
{noformat}
1 synchronized SyncFuture reset(final long sequence, Span span) {
2if (t != null && t != Thread.currentThread()) throw new
IllegalStateException();
3t = Thread.currentThread();
4if (!isDone()) throw new IllegalStateException("" + sequence + " " +
Thread.currentThread());
5this.doneSequence = NOT_DONE;
6this.ringBufferSequence = sequence;
7this.span = span;
8this.throwable = null;
9return this;
10 }
{noformat}
We can see, there are guards against overwriting un-finished sync futures with
checks on ‘current thread’ (L2) and !isDone() (L4). These are not tight checks,
consider the following sequence of interleaved actions that can result in a
deadlock..
# RPC handler#1 → WAL sync - SyncFuture#1 (from ThreadLocalCache) → Add to
RingBuffer - state: NOT_DONE
# RingBufferConsumer#onEvent() → SyncFuture#1 dispatch → old state: NOT_DONE,
new state: DONE (via releaseSyncFuture() in SyncRunner.run())
# RPCHandler#1 (reused for another region operation) → ThreadLocal
SyncFuture#1 NOT_DONE (via reset()) (*this goes through because isDone() now
returns true and L2 always returns true because we reuse the ThreadLocal
instance*).
# RingBufferConsumer#onEvent() - attainSafePoint() -> works on the
overwritten sync future state (from step 3)
# DEADLOCK === (as the sync future remains in the state forever)
The problem here is that once a SyncFuture is marked DONE, it is eligible for
reuse. Even though ring buffer still has references to it and is checking on it
to attain a safe point, in the background another handler can just overwrite it
resulting in ring buffer operating on a wrong future and deadlocking the
system. Very subtle bug.. I'm able to reproduce this in a carefully crafted
unit test (attached). I have a fix solves this problem, will create a PR for
review soon.
was (Author: bharathv):
Obtained a heap dump and poked around quite a bit and observed that in-memory
state is corrupted. After analyzing it and eyeballing the code (long story
short), came up with the following theory...
This is the current implementation of SyncFuture#reset()
{noformat}
1 synchronized SyncFuture reset(final long sequence, Span span) {
2if (t != null && t != Thread.currentThread()) throw new
IllegalStateException();
3t = Thread.currentThread();
4if (!isDone()) throw new IllegalStateException("" + sequence + " " +
Thread.currentThread());
5this.doneSequence = NOT_DONE;
6this.ringBufferSequence = sequence;
7this.span = span;
8this.throwable = null;
9return this;
10 }
{noformat}
We can see, there are guards against overwriting un-finished sync futures with
checks on ‘current thread’ (L2) and !isDone() (L4). These are not tight checks,
consider the following sequence of interleaved actions that can result in a
deadlock..
# RPC handler#1 → WAL sync - SyncFuture#1 (from ThreadLocalCache) → Add to
RingBuffer - state: NOT_DONE
# RingBufferConsumer#onEvent() → SyncFuture#1 dispatch → old state: NOT_DONE,
new state: DONE (via releaseSyncFuture() in SyncRunner.run())
# RPCHandler#1 (reused for another region operation) → ThreadLocal
SyncFuture#1 NOT_DONE (via reset()) (*this goes through because isDone() now
returns true and L2 always returns true because we reuse the ThreadLocal
instance*).
# RingBufferConsumer#onEvent() - attainSafePoint() -> works on the
overwritten sync future state (from step 3)
# DEADLOCK === (as the sync future remains in the state forever)
The problem here is that once a SyncFuture is marked DONE, it is eligible for
reuse. Even though ring buffer still has references to it and is checking on it
to attain a safe point, in the background another handler can just overwrite it
resulting in ring buffer operating on a wrong future and deadlocking the
system. Very subtle bug.. I'm able to reproduce this in a carefully crafted
unit test (attached). I have a fix solves this problem, will create a PR for
review soon.
> FSHLog WAL lockup with sync future reuse [RS deadlock]
> --
>
> Key: HBASE-25984
> URL: https://issues.apache.org/jira/browse/HBASE-25984
> Project: HBase
> Issue Type: Bug
> Components: regionserver, wal
>Affects Versions: 3.0.0-alpha-1, 1.7.0, 2.5.0, 2.4.5
>
