pybind/ceph_daemon: use small chunk for recv #13804
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tchaikov
previously requested changes
Mar 7, 2017
src/pybind/ceph_daemon.py
Outdated
| @@ -43,9 +43,12 @@ def do_sockio(path, cmd_bytes): | |||
| l, = struct.unpack(">I", len_str) | |||
| sock_ret = b'' | |||
|
|
|||
| got = 0 | |||
| got = long(0) | |||
There was a problem hiding this comment.
this breaks the python3 support.
src/pybind/ceph_daemon.py
Outdated
| bit = sock.recv(l - got) | ||
| # recv only accept signed int, i.e max 2GB | ||
| # walk around by limit each recv to 128M | ||
| want = min(l - got, 128*1024*1024) |
There was a problem hiding this comment.
i'd suggest define a "relatively small power of 2, for example, 4096." [1], i.e.
READ_CHUNK_SIZE = 4096and use it instead,
xiaoxichen
force-pushed
the
fix_adminsocket
branch
2 times, most recently
from
a9164e3
to
367a035
Compare
xiaoxichen
changed the title
|
@tchaikov addressed |
tchaikov
reviewed
Mar 8, 2017
src/pybind/ceph_daemon.py
Outdated
| @@ -45,7 +46,10 @@ def do_sockio(path, cmd_bytes): | |||
|
|
|||
| got = 0 | |||
| while got < l: | |||
| bit = sock.recv(l - got) | |||
| # recv only accept signed int, i.e max 2GB | |||
xiaoxichen
force-pushed
the
fix_adminsocket
branch
from
367a035
to
f223309
Compare
tchaikov
reviewed
Mar 9, 2017
src/pybind/ceph_daemon.py
Outdated
| @@ -45,7 +46,10 @@ def do_sockio(path, cmd_bytes): | |||
|
|
|||
| got = 0 | |||
| while got < l: | |||
| bit = sock.recv(l - got) | |||
| # recv() receives signed int, i.e max 2GB | |||
| # walk around by request max READ_CHUNK_SIZE per call. | |||
There was a problem hiding this comment.
workaround by capping, might want to update the commit message also.
xiaoxichen
force-pushed
the
fix_adminsocket
branch
from
f223309
to
c1b98c6
Compare
tchaikov
reviewed
Mar 9, 2017
src/pybind/ceph_daemon.py
Outdated
| @@ -45,7 +46,10 @@ def do_sockio(path, cmd_bytes): | |||
|
|
|||
| got = 0 | |||
| while got < l: | |||
| bit = sock.recv(l - got) | |||
| # recv() receives signed int, i.e max 2GB | |||
| # walkaround by capping READ_CHUNK_SIZE per call. | |||
There was a problem hiding this comment.
done, thanks for the english lesson :)
socket.recv(bufsize) accept signed int in python, so if we want to load huge data (mds -> cache_dump is an instance) from admin socket , an EOVERFLOW exception will be throw. Workaround by capping READ_CHUNK_SIZE(4096) bytes each call. Signed-off-by: Xiaoxi Chen <xiaoxchen@ebay.com>
xiaoxichen
force-pushed
the
fix_adminsocket
branch
from
c1b98c6
to
4e8a19b
Compare
tchaikov
approved these changes
Mar 10, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
socket.recv(bufsize) accept signed int in python, so if
we want to load huge data (mds -> cache_dump is an instance)
from admin socket , an EOVERFLOW exception will be throw.
Walk around this by only get 4096 bytes for each call
Signed-off-by: Xiaoxi Chen xiaoxchen@ebay.com