You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Andrew Wong (Code Review)" <ge...@cloudera.org> on 2020/03/28 10:31:03 UTC
[kudu-CR] env: add a fifo class
Andrew Wong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15584
Change subject: env: add a fifo class
......................................................................
env: add a fifo class
This adds a basic Fifo class that encapsulates the usage of mkfifo and
associated functionality. My immediate need is just to return the FDs of
the opened fifo, so the interface is quite bare. That said, this will be
good enough to move the SubprocessServer away from using the process
stdout FD for IPC.
Change-Id: I23920457d695b84509937694daea53d61f445e27
---
M src/kudu/util/env-test.cc
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
3 files changed, 132 insertions(+), 21 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/15584/1
--
To view, visit http://gerrit.cloudera.org:8080/15584
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I23920457d695b84509937694daea53d61f445e27
Gerrit-Change-Number: 15584
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
[kudu-CR] env: add a fifo class
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/15584
to look at the new patch set (#2).
Change subject: env: add a fifo class
......................................................................
env: add a fifo class
This adds a basic Fifo class and a PosixFifo variant that encapsulates
the usage of mkfifo and associated functionality. My immediate need is
just to return the FDs of the opened fifo, so the interface is quite
bare. That said, this will be good enough to move the SubprocessServer
away from using the process stdout FD for IPC.
Change-Id: I23920457d695b84509937694daea53d61f445e27
---
M src/kudu/util/env-test.cc
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
3 files changed, 130 insertions(+), 21 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/15584/2
--
To view, visit http://gerrit.cloudera.org:8080/15584
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I23920457d695b84509937694daea53d61f445e27
Gerrit-Change-Number: 15584
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] env: add a fifo class
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/15584 )
Change subject: env: add a fifo class
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/15584
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I23920457d695b84509937694daea53d61f445e27
Gerrit-Change-Number: 15584
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Mon, 30 Mar 2020 18:33:07 +0000
Gerrit-HasComments: No
[kudu-CR] env: add a fifo class
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15584 )
Change subject: env: add a fifo class
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/15584/3/src/kudu/util/env_posix.cc
File src/kudu/util/env_posix.cc:
http://gerrit.cloudera.org:8080/#/c/15584/3/src/kudu/util/env_posix.cc@357
PS3, Line 357: if (f == -1) {
: return Status::IOError(Substitute("Error opening for $0: $1", reason, filename));
> Done
Not done? I see you've changed NewFifo but not DoOpen here.
--
To view, visit http://gerrit.cloudera.org:8080/15584
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I23920457d695b84509937694daea53d61f445e27
Gerrit-Change-Number: 15584
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Mon, 30 Mar 2020 17:33:51 +0000
Gerrit-HasComments: Yes
[kudu-CR] env: add a fifo class
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/15584
to look at the new patch set (#3).
Change subject: env: add a fifo class
......................................................................
env: add a fifo class
This adds a basic Fifo class and a PosixFifo variant that encapsulates
the usage of mkfifo and associated functionality. My immediate need is
just to return the FDs of the opened fifo, so the interface is quite
bare. That said, this will be good enough to move the SubprocessServer
away from using the process stdout FD for IPC.
Change-Id: I23920457d695b84509937694daea53d61f445e27
---
M src/kudu/util/env-test.cc
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
3 files changed, 132 insertions(+), 21 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/15584/3
--
To view, visit http://gerrit.cloudera.org:8080/15584
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I23920457d695b84509937694daea53d61f445e27
Gerrit-Change-Number: 15584
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] env: add a fifo class
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15584 )
Change subject: env: add a fifo class
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/15584/3/src/kudu/util/env_posix.cc
File src/kudu/util/env_posix.cc:
http://gerrit.cloudera.org:8080/#/c/15584/3/src/kudu/util/env_posix.cc@357
PS3, Line 357: if (f == -1) {
: return Status::IOError(Substitute("Error opening for $0: $1", reason, filename));
> Can we return the results of the IOError() function here instead?
Done
--
To view, visit http://gerrit.cloudera.org:8080/15584
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I23920457d695b84509937694daea53d61f445e27
Gerrit-Change-Number: 15584
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Mon, 30 Mar 2020 03:26:30 +0000
Gerrit-HasComments: Yes
[kudu-CR] env: add a fifo class
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15584 )
Change subject: env: add a fifo class
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/15584/3/src/kudu/util/env_posix.cc
File src/kudu/util/env_posix.cc:
http://gerrit.cloudera.org:8080/#/c/15584/3/src/kudu/util/env_posix.cc@357
PS3, Line 357: if (f == -1) {
: return Status::IOError(Substitute("Error opening for $0: $1", reason, filename));
Can we return the results of the IOError() function here instead?
--
To view, visit http://gerrit.cloudera.org:8080/15584
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I23920457d695b84509937694daea53d61f445e27
Gerrit-Change-Number: 15584
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Sun, 29 Mar 2020 23:12:40 +0000
Gerrit-HasComments: Yes
[kudu-CR] env: add a fifo class
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/15584
to look at the new patch set (#4).
Change subject: env: add a fifo class
......................................................................
env: add a fifo class
This adds a basic Fifo class and a PosixFifo variant that encapsulates
the usage of mkfifo and associated functionality. My immediate need is
just to return the FDs of the opened fifo, so the interface is quite
bare. That said, this will be good enough to move the SubprocessServer
away from using the process stdout FD for IPC.
Change-Id: I23920457d695b84509937694daea53d61f445e27
---
M src/kudu/util/env-test.cc
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
3 files changed, 132 insertions(+), 21 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/15584/4
--
To view, visit http://gerrit.cloudera.org:8080/15584
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I23920457d695b84509937694daea53d61f445e27
Gerrit-Change-Number: 15584
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] env: add a fifo class
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15584 )
Change subject: env: add a fifo class
......................................................................
Patch Set 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/15584/3/src/kudu/util/env_posix.cc
File src/kudu/util/env_posix.cc:
http://gerrit.cloudera.org:8080/#/c/15584/3/src/kudu/util/env_posix.cc@357
PS3, Line 357: if (f == -1) {
: return IOError(Substitute("Error opening for $0: $1", reason, filename), errno);
> Not done? I see you've changed NewFifo but not DoOpen here.
Done
--
To view, visit http://gerrit.cloudera.org:8080/15584
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I23920457d695b84509937694daea53d61f445e27
Gerrit-Change-Number: 15584
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Mon, 30 Mar 2020 18:16:33 +0000
Gerrit-HasComments: Yes
[kudu-CR] env: add a fifo class
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15584 )
Change subject: env: add a fifo class
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/15584
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I23920457d695b84509937694daea53d61f445e27
Gerrit-Change-Number: 15584
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Mon, 30 Mar 2020 18:17:12 +0000
Gerrit-HasComments: No
[kudu-CR] env: add a fifo class
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/15584
to look at the new patch set (#5).
Change subject: env: add a fifo class
......................................................................
env: add a fifo class
This adds a basic Fifo class and a PosixFifo variant that encapsulates
the usage of mkfifo and associated functionality. My immediate need is
just to return the FDs of the opened fifo, so the interface is quite
bare. That said, this will be good enough to move the SubprocessServer
away from using the process stdout FD for IPC.
Change-Id: I23920457d695b84509937694daea53d61f445e27
---
M src/kudu/util/env-test.cc
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
3 files changed, 132 insertions(+), 21 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/15584/5
--
To view, visit http://gerrit.cloudera.org:8080/15584
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I23920457d695b84509937694daea53d61f445e27
Gerrit-Change-Number: 15584
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] env: add a fifo class
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15584 )
Change subject: env: add a fifo class
......................................................................
env: add a fifo class
This adds a basic Fifo class and a PosixFifo variant that encapsulates
the usage of mkfifo and associated functionality. My immediate need is
just to return the FDs of the opened fifo, so the interface is quite
bare. That said, this will be good enough to move the SubprocessServer
away from using the process stdout FD for IPC.
Change-Id: I23920457d695b84509937694daea53d61f445e27
Reviewed-on: http://gerrit.cloudera.org:8080/15584
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Reviewed-by: Hao Hao <ha...@cloudera.com>
Tested-by: Kudu Jenkins
---
M src/kudu/util/env-test.cc
M src/kudu/util/env.h
M src/kudu/util/env_posix.cc
3 files changed, 132 insertions(+), 21 deletions(-)
Approvals:
Adar Dembo: Looks good to me, approved
Hao Hao: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/15584
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I23920457d695b84509937694daea53d61f445e27
Gerrit-Change-Number: 15584
Gerrit-PatchSet: 6
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)