Skip to content

Fixed race condition intruduced managed ledger addEntry introduced in #1521 - #1548

Merged
sijie merged 1 commit into
apache:masterfrom
merlimat:fix-add-entry
Apr 12, 2018
Merged

Fixed race condition intruduced managed ledger addEntry introduced in #1521#1548
sijie merged 1 commit into
apache:masterfrom
merlimat:fix-add-entry

Conversation

@merlimat

Copy link
Copy Markdown
Contributor

Motivation

The change in #1521 introduced a race between multiple writers since the enqueing and sending operation were split between 2 threads. Fixing here by ensuring a single thread does both operation in same order.

This problem was surfacing in tests failing with this error:

21:12:17.989 [bookkeeper-ml-workers-OrderedExecutor-13-0:org.apache.bookkeeper.common.util.SafeRunnable@38] ERROR org.apache.bookkeeper.common.util.SafeRunnable - Unexpected throwable caught 
java.lang.IllegalArgumentException: null
	at com.google.common.base.Preconditions.checkArgument(Preconditions.java:108) ~[guava-20.0.jar:?]
	at org.apache.bookkeeper.mledger.impl.OpAddEntry.safeRun(OpAddEntry.java:142) ~[classes/:?]
	at org.apache.bookkeeper.common.util.SafeRunnable.run(SafeRunnable.java:36) [bookkeeper-server-shaded-4.7.0-SNAPSHOT.jar:4.7.0-SNAPSHOT]
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142) [?:1.8.0_121]
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617) [?:1.8.0_121]
	at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30) [netty-all-4.1.21.Final.jar:4.1.21.Final]
	at java.lang.Thread.run(Thread.java:745) [?:1.8.0_121]

@merlimat merlimat added the type/bug The PR fixed a bug or issue reported a bug label Apr 11, 2018
@merlimat merlimat added this to the 2.0.0-incubating milestone Apr 11, 2018
@merlimat merlimat self-assigned this Apr 11, 2018
@merlimat
merlimat requested review from jai1, rdhabalia and sijie April 11, 2018 04:44
@sijie
sijie merged commit aff6c04 into apache:master Apr 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type/bug The PR fixed a bug or issue reported a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants