Revert "Issue #1791: Read Submission should bypass OSE Threads"

[eolivelli]

This reverts commit 6b99ff7.

Descriptions of the changes in this PR:
Revert #1792 "Issue #1791: Read Submission should bypass OSE Threads"

Motivation

See #3104

[eolivelli]

@diegosalvi this change is also important for HerdDB and DistributedLog

[lhotari]

There's #3105 as a possible solution for mitigating the performance issue which was the original motivation for the PR which is reverted by this PR.

[diegosalvi]

@eolivelli interesting, thank you. We'll test disabling netty recycler as stated in apache/pulsar#14436 for now

[dlg99]

@Ghatage FYI

[dlg99]

@congbobo184 @lhotari As I understand, there is no confirmation that reverting this PR actually fixes the problem reported in Pulsar.

If this problem is related to calling netty's recycle from another thread (which netty had and fixed) I'd love to see something that actually proves the theory so netty can fix it.

I don't see anything in PendingReadOp that requires initiate() and readEntryComplete() to run on the same thread for the read-only ledger. I can be wrong, please help me understand what I am missing. readEntryComplete() still runs on the OSE.

What you see in the log mentioned in the comment might happen if:

• request took longer than speculative retry time
• speculative read got submitted
• both original request and the speculative retry succeeded

It is possible that handling of this case somehow corrupts recyclable entry though i don't see yet where it could happen (callbacks should be handled on the same OSE thread etc)

I need more proof to confirm this actually fixes the problem and not just reverts some PR that touches code you are suspecting.

[jvrao]

+1 on @dlg99 's comment. I could not understand where exactly the problem and how it could get addressed by making create and recycle run in the same thread. You could always have outstanding read responses after erroring out. But even if that happens, what exactly the error/corruption that is visible to the client?

[congbobo184]

#3110 will fix it

[lhotari]

You could always have outstanding read responses after erroring out. But even if that happens, what exactly the error/corruption that is visible to the client?

@jvrao The issue description is #3104 and there are references to a Pulsar issue with a lot of troubleshooting comments. @congbobo184 has also summarized the behavior in the PR #3110 description.

[eolivelli]

Closing for now

[hangc0276]

The root cause of #3104 is not caused by the thread safety problem. And the isolation thread pool for cold and hot data reading will greatly reduce thread switching overhead and make the throughput more stable.

This issue has been fixed by #3110