Skip to content

channeld: prioritize read from peer over write to peer - #2297

Merged
cdecker merged 3 commits into
masterfrom
unknown repository
Jan 29, 2019
Merged

channeld: prioritize read from peer over write to peer#2297
cdecker merged 3 commits into
masterfrom
unknown repository

Conversation

@ghost

@ghost ghost commented Jan 26, 2019

Copy link
Copy Markdown

channeld: prioritize read from peer over write to peer

This solves (or at least reduces probability of) a deadlock in channeld
when there is lot of gossip traffic, see issue #2286. That issue is
almost identical to #1943 (deadlock in openingd) and so is the fix.

I did a simple test, monitoring socket traffic Recv-Q and Send-Q at 1 second interval, using ss -4 -t -p | egrep "lightning|State" during startup. The node has one channel with satoshisplace, so I don't know if this problem is specific to that node, but this solved the deadlock for me.

Result after this commit:

State   Recv-Q Send-Q Local Address:Port                 Peer Address:Port                
ESTAB 0 107 192.168.1.166:44902 88.98.213.235:9735 users:(("lightning_conne",pid=30591,fd=6))
ESTAB 0 106833 192.168.1.166:44902 88.98.213.235:9735 users:(("lightning_chann",pid=30628,fd=3))
ESTAB 0 2242 192.168.1.166:44902 88.98.213.235:9735 users:(("lightning_chann",pid=30628,fd=3))
ESTAB 0 118931 192.168.1.166:44902 88.98.213.235:9735 users:(("lightning_chann",pid=30628,fd=3))
ESTAB 0 192867 192.168.1.166:44902 88.98.213.235:9735 users:(("lightning_chann",pid=30628,fd=3))
ESTAB 0 60478 192.168.1.166:44902 88.98.213.235:9735 users:(("lightning_chann",pid=30628,fd=3))
ESTAB 0 191260 192.168.1.166:44902 88.98.213.235:9735 users:(("lightning_chann",pid=30628,fd=3))
ESTAB 0 71770 192.168.1.166:44902 88.98.213.235:9735 users:(("lightning_chann",pid=30628,fd=3))
ESTAB 0 0 192.168.1.166:44902 88.98.213.235:9735 users:(("lightning_chann",pid=30628,fd=3))
ESTAB 0 65781 192.168.1.166:44902 88.98.213.235:9735 users:(("lightning_chann",pid=30628,fd=3))
ESTAB 0 68496 192.168.1.166:44902 88.98.213.235:9735 users:(("lightning_chann",pid=30628,fd=3))
ESTAB 0 622707 192.168.1.166:44902 88.98.213.235:9735 users:(("lightning_chann",pid=30628,fd=3))
ESTAB 0 2916 192.168.1.166:44902 88.98.213.235:9735 users:(("lightning_chann",pid=30628,fd=3))
ESTAB 0 84782 192.168.1.166:44902 88.98.213.235:9735 users:(("lightning_chann",pid=30628,fd=3))
ESTAB 0 24917 192.168.1.166:44902 88.98.213.235:9735 users:(("lightning_chann",pid=30628,fd=3))
ESTAB 0 36609 192.168.1.166:44902 88.98.213.235:9735 users:(("lightning_chann",pid=30628,fd=3))
ESTAB 0 10866 192.168.1.166:44902 88.98.213.235:9735 users:(("lightning_chann",pid=30628,fd=3))
ESTAB 0 794 192.168.1.166:44902 88.98.213.235:9735 users:(("lightning_chann",pid=30628,fd=3))
ESTAB 0 76226 192.168.1.166:44902 88.98.213.235:9735 users:(("lightning_chann",pid=30628,fd=3))
ESTAB 1245 5648 192.168.1.166:44902 88.98.213.235:9735 users:(("lightning_chann",pid=30628,fd=3))
ESTAB 0 630629 192.168.1.166:44902 88.98.213.235:9735 users:(("lightning_chann",pid=30628,fd=3))
ESTAB 0 626984 192.168.1.166:44902 88.98.213.235:9735 users:(("lightning_chann",pid=30628,fd=3))
ESTAB 0 486528 192.168.1.166:44902 88.98.213.235:9735 users:(("lightning_chann",pid=30628,fd=3))
ESTAB 0 373345 192.168.1.166:44902 88.98.213.235:9735 users:(("lightning_chann",pid=30628,fd=3))
ESTAB 0 0 192.168.1.166:44902 88.98.213.235:9735 users:(("lightning_chann",pid=30628,fd=3))
ESTAB 0 0 192.168.1.166:44902 88.98.213.235:9735 users:(("lightning_chann",pid=30628,fd=3))
ESTAB 0 0 192.168.1.166:44902 88.98.213.235:9735 users:(("lightning_chann",pid=30628,fd=3))
ESTAB 0 0 192.168.1.166:44902 88.98.213.235:9735 users:(("lightning_chann",pid=30628,fd=3))
ESTAB 0 0 192.168.1.166:44902 88.98.213.235:9735 users:(("lightning_chann",pid=30628,fd=3))
...

Result before this commit:

State   Recv-Q Send-Q Local Address:Port                 Peer Address:Port                
ESTAB 133145 124528 192.168.1.166:45256 88.98.213.235:9735 users:(("lightning_chann",pid=31325,fd=3))
ESTAB 249555 162176 192.168.1.166:45256 88.98.213.235:9735 users:(("lightning_chann",pid=31325,fd=3))
ESTAB 250835 193264 192.168.1.166:45256 88.98.213.235:9735 users:(("lightning_chann",pid=31325,fd=3))
ESTAB 250835 193264 192.168.1.166:45256 88.98.213.235:9735 users:(("lightning_chann",pid=31325,fd=3))
ESTAB 250835 193264 192.168.1.166:45256 88.98.213.235:9735 users:(("lightning_chann",pid=31325,fd=3))
ESTAB 250835 193264 192.168.1.166:45256 88.98.213.235:9735 users:(("lightning_chann",pid=31325,fd=3))
...

Simon Vrouwe and others added 2 commits January 27, 2019 10:08
Simon Vrouwe
channeld: prioritize read from peer over (read from gossipd and) writ…
1ebe311 
…e to peer

This solves (or at least reduces probability of) a deadlock in channeld
when there is lot of gossip traffic, see issue #2286. That issue is
almost identical to #1943 (deadlock in openingd) and so is the fix.
@rustyrussell
common: handle peer input before gossipd input (for closingd, openingd)
f0d4f05 
Similar to the previous "handle peer input before gossip input", this
fixes similar potential deadlock for closingd and openingd which use
peer_or_gossip_sync_read.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell

rustyrussell commented Jan 29, 2019

Copy link
Copy Markdown
Collaborator

Added same fix for the other daemons....

@rustyrussell
CHANGELOG.md: document deadlock fix.
b6b9246 
Useful if others hit it.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@cdecker

cdecker commented Jan 29, 2019

Copy link
Copy Markdown
Member

ACK b6b9246

@cdecker
cdecker merged commit f1a837e into ElementsProject:master Jan 29, 2019
@ghost
ghost deleted the 26jan_channeld_read_peer_before_write branch February 4, 2019 15:48
@jnewbery jnewbery mentioned this pull request Jun 4, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@rustyrussell @cdecker