Skip to content

Problem: abruptly closed sockets still in memory#2408

Merged
shahbazn merged 1 commit intobigchaindb:masterfrom
vrde:fix-websocket-exception
Jul 26, 2018
Merged

Problem: abruptly closed sockets still in memory#2408
shahbazn merged 1 commit intobigchaindb:masterfrom
vrde:fix-websocket-exception

Conversation

@vrde
Copy link
Copy Markdown
Contributor

@vrde vrde commented Jul 25, 2018

Solution: Catch the proper exception and remove socket from memory.

The problem looks like this in the console:

[2018-07-25 11:54:06] [WARNING] (asyncio) socket.send() raised exception. (bigchaindb_ws - pid: 86105)
[2018-07-25 11:54:06] [WARNING] (asyncio) socket.send() raised exception. (bigchaindb_ws - pid: 86105)
[2018-07-25 11:54:06] [WARNING] (asyncio) socket.send() raised exception. (bigchaindb_ws - pid: 86105)
[2018-07-25 11:54:06] [WARNING] (asyncio) socket.send() raised exception. (bigchaindb_ws - pid: 86105)
[2018-07-25 11:54:06] [WARNING] (asyncio) socket.send() raised exception. (bigchaindb_ws - pid: 86105)
[2018-07-25 11:54:06] [WARNING] (asyncio) socket.send() raised exception. (bigchaindb_ws - pid: 86105)
[2018-07-25 11:54:06] [WARNING] (asyncio) socket.send() raised exception. (bigchaindb_ws - pid: 86105)
[2018-07-25 11:54:06] [WARNING] (asyncio) socket.send() raised exception. (bigchaindb_ws - pid: 86105)
[2018-07-25 11:54:06] [WARNING] (asyncio) socket.send() raised exception. (bigchaindb_ws - pid: 86105)
[2018-07-25 11:54:06] [WARNING] (asyncio) socket.send() raised exception. (bigchaindb_ws - pid: 86105)
[2018-07-25 11:54:06] [WARNING] (asyncio) socket.send() raised exception. (bigchaindb_ws - pid: 86105)
[2018-07-25 11:54:06] [WARNING] (asyncio) socket.send() raised exception. (bigchaindb_ws - pid: 86105)

Note for myself: asyncio and exceptions goes well like 🐈 and 🐕

Solution: Catch the proper exception and remove socket from memory.
@codecov-io
Copy link
Copy Markdown

codecov-io commented Jul 25, 2018

Codecov Report

Merging #2408 into master will decrease coverage by 0.11%.
The diff coverage is 25%.

@@            Coverage Diff             @@
##           master    #2408      +/-   ##
==========================================
- Coverage    86.9%   86.79%   -0.12%     
==========================================
  Files          38       38              
  Lines        2169     2173       +4     
==========================================
+ Hits         1885     1886       +1     
- Misses        284      287       +3

@vrde vrde requested a review from shahbazn July 25, 2018 12:39
logger.debug('Websocket exception: %s', str(e))
break
except CancelledError as e:
logger.debug('Websocket closed')
Copy link
Copy Markdown
Contributor

@shahbazn shahbazn Jul 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this info will be useful on production setup as well. shouldn't this be info level instead of debug?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How much do you care about a disconnected socket?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

such failures might not be easily reproducible in dev so I think its important to capture it in prod

@vrde
Copy link
Copy Markdown
Contributor Author

vrde commented Jul 25, 2018

Testing this is honestly a big PITA. Even if code coverage is -0.12%, we can merge it.

@shahbazn shahbazn merged commit 2087f70 into bigchaindb:master Jul 26, 2018
@vrde vrde deleted the fix-websocket-exception branch July 26, 2018 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants