Go-datakit handle large values when reading / writing in 9db #292
Conversation
simonferquel
force-pushed
the
large_value_handling
branch
from
0f3582d
to
d76341b
Compare
simonferquel
force-pushed
the
large_value_handling
branch
from
33db451
to
5e1e062
Compare
|
@simonferquel Can I merge this PR now that the 2 other ones have been merged? |
|
@simonferquel ping? |
|
The most important pr was closed without being merged. We have yet to find a solution both fixing the problem and satisfying Stephen. |
|
This seems to be blocked on docker-archive/go-p9p#29 (or possibly docker-archive/go-p9p#30). |
You have yet to propose a solution that leverages Go's IO model. It also lacks a proper bug report and reproduction of the issue. So far, I've taken time out of my regular tasks to characterize and propose a solution for the write side. I have characterized the problem for the read side but have not had time to create a minimally invasive solution. I'm still waiting on confirmation from @simonferquel that docker-archive/go-p9p#30 addresses the write side of the issue. If the issues with the existing proposal (docker-archive/go-p9p#29) are not yet understood, please reach out so we can resolve this. |
|
docker-archive/go-p9p#30 should now address the described issues. |
simonferquel
force-pushed
the
large_value_handling
branch
from
5e1e062
to
ed1051e
Compare
simonferquel
force-pushed
the
large_value_handling
branch
from
ed1051e
to
c81f5ca
Compare
|
We are almost there : current package runs alongside docker-archive/go-p9p#30. This PR is almost ready to merge. |
|
Is this ready for merging now or are we still waiting for something? |
simonferquel
changed the title
|
Good to merge (just need to update go-p9p to the latest master version to build and run correctly). Be careful when vendoring into piñata. |
@simonferquel : Could you clarify: is this good to merge, or are we waiting to update go-p9p? |
|
@djs55 @simonferquel could we close and/or merge this PR? I am not sure anymore which fork is used in Docker for Mac today ... |
…the fact that p9p can do partial read / writes with large values Signed-off-by: Simon Ferquel <[email protected]>
Signed-off-by: Simon Ferquel <[email protected]>
Signed-off-by: Simon Ferquel <[email protected]>
Signed-off-by: Simon Ferquel <[email protected]>
Signed-off-by: Simon Ferquel <[email protected]>
Signed-off-by: Simon Ferquel <[email protected]>
Signed-off-by: Simon Ferquel <[email protected]>
Signed-off-by: Simon Ferquel <[email protected]>
simonferquel
force-pushed
the
large_value_handling
branch
from
0d174d6
to
cb0fe49
Compare
|
Thanks for the rebase! As discussed with @simonferquel and @ebriney I am merging this. @djs55 do you know why cb0fe49 is needed? I guess this is related to the changes that you made recently in the config code. Can you check what is going on? |
CHANGES: - all: update to latest version of alcotest, conduit, session, ocaml-github, ocaml-github-hooks and cohttp (moby/datakit#612, @samoht and @djs55) - github: make `User.t` abstract (moby/datakit#594, @samoht) - github: turn `Webhook.events` into a promise (moby/datakit#598, @samoht) - github: add a `Comment` module to model PR and issue comments (moby/datakit#595, @samoht) - github: change `PR.owner` to be of type `User.t` (moby/datakit#599, @samoht) - github-bridge: add the ability to sync PR's coments (moby/datakit#595, @samoht) - go-client: handle large values when reading / writing in 9db (moby/datakit#292, @simonferquel) - go-client: fix the handling of defaults over upgrade (moby/datakit#605, @djs55) - go-client: improve transaction API (moby/datakit#606, @djs55)
CHANGES: - all: update to latest version of alcotest, conduit, session, ocaml-github, ocaml-github-hooks and cohttp (moby/datakit#612, @samoht and @djs55) - github: make `User.t` abstract (moby/datakit#594, @samoht) - github: turn `Webhook.events` into a promise (moby/datakit#598, @samoht) - github: add a `Comment` module to model PR and issue comments (moby/datakit#595, @samoht) - github: change `PR.owner` to be of type `User.t` (moby/datakit#599, @samoht) - github-bridge: add the ability to sync PR's coments (moby/datakit#595, @samoht) - go-client: handle large values when reading / writing in 9db (moby/datakit#292, @simonferquel) - go-client: fix the handling of defaults over upgrade (moby/datakit#605, @djs55) - go-client: improve transaction API (moby/datakit#606, @djs55)
CHANGES: - all: update to latest version of alcotest, conduit, session, ocaml-github, ocaml-github-hooks and cohttp (moby/datakit#612, @samoht and @djs55) - github: make `User.t` abstract (moby/datakit#594, @samoht) - github: turn `Webhook.events` into a promise (moby/datakit#598, @samoht) - github: add a `Comment` module to model PR and issue comments (moby/datakit#595, @samoht) - github: change `PR.owner` to be of type `User.t` (moby/datakit#599, @samoht) - github-bridge: add the ability to sync PR's coments (moby/datakit#595, @samoht) - go-client: handle large values when reading / writing in 9db (moby/datakit#292, @simonferquel) - go-client: fix the handling of defaults over upgrade (moby/datakit#605, @djs55) - go-client: improve transaction API (moby/datakit#606, @djs55)
CHANGES: - all: update to latest version of alcotest, conduit, session, ocaml-github, ocaml-github-hooks and cohttp (moby/datakit#612, @samoht and @djs55) - github: make `User.t` abstract (moby/datakit#594, @samoht) - github: turn `Webhook.events` into a promise (moby/datakit#598, @samoht) - github: add a `Comment` module to model PR and issue comments (moby/datakit#595, @samoht) - github: change `PR.owner` to be of type `User.t` (moby/datakit#599, @samoht) - github-bridge: add the ability to sync PR's coments (moby/datakit#595, @samoht) - go-client: handle large values when reading / writing in 9db (moby/datakit#292, @simonferquel) - go-client: fix the handling of defaults over upgrade (moby/datakit#605, @djs55) - go-client: improve transaction API (moby/datakit#606, @djs55)
Requires docker-archive/go-p9p#27 and docker-archive/go-p9p#28
When dealing with messages larger than the negociated MSize value, go-p9p truncates writes and makes partial reads (without the 2 prs, it just fails). Clients must be aware of that and use the returned n value to handle that gracefully.
This PR introduces an io.Reader / io.Writer compliant wrapper that handles that correctly (and use it for Snapshot.Read and Transaction.Write operations)