You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Lars Volker (Code Review)" <ge...@cloudera.org> on 2018/06/19 22:32:30 UTC
[Impala-ASF-CR] IMPALA-5873: Check for existence of sync file range()
Hello Michael Ho, Impala Public Jenkins,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/10762
to review the following change.
Change subject: IMPALA-5873: Check for existence of sync_file_range()
......................................................................
IMPALA-5873: Check for existence of sync_file_range()
On some legacy platforms, the system call sync_file_range()
is not defined. Some Kudu code calls it unconditionally
on Linux platforms. Let's check for its existence and use
the workaround already defined for non-Linux platforms.
Testing done: ran packaging build on Centos55. Failed consistently before.
Change-Id: Ib4f9409ca3a519b8b9eb5ca91a3c2a6c7c25e34f
Reviewed-on: http://gerrit.cloudera.org:8080/7923
Reviewed-by: Michael Ho <kw...@cloudera.com>
Tested-by: Impala Public Jenkins
---
M be/src/kudu/util/env_posix.cc
1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/62/10762/1
--
To view, visit http://gerrit.cloudera.org:8080/10762
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib4f9409ca3a519b8b9eb5ca91a3c2a6c7c25e34f
Gerrit-Change-Number: 10762
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
[Impala-ASF-CR] IMPALA-5873: Check for existence of sync file range()
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/10762 )
Change subject: IMPALA-5873: Check for existence of sync_file_range()
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10762/1/be/src/kudu/util/env_posix.cc
File be/src/kudu/util/env_posix.cc:
http://gerrit.cloudera.org:8080/#/c/10762/1/be/src/kudu/util/env_posix.cc@757
PS1, Line 757: defined(HAVE_SYNC_FILE_RANGE)
> Applicable to Kudu too ?!
Looks to me like we could add it there. Kudu doesn't seem to use these cmake-defines, but we could add that functionality.
This was a change done by yourself: https://gerrit.cloudera.org/#/c/7923/
Do you recall why we didn't push it upstream?
--
To view, visit http://gerrit.cloudera.org:8080/10762
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4f9409ca3a519b8b9eb5ca91a3c2a6c7c25e34f
Gerrit-Change-Number: 10762
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Fri, 22 Jun 2018 00:36:10 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5873: Check for existence of sync file range()
Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/10762 )
Change subject: IMPALA-5873: Check for existence of sync_file_range()
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10762/1/be/src/kudu/util/env_posix.cc
File be/src/kudu/util/env_posix.cc:
http://gerrit.cloudera.org:8080/#/c/10762/1/be/src/kudu/util/env_posix.cc@757
PS1, Line 757: defined(HAVE_SYNC_FILE_RANGE)
Applicable to Kudu too ?!
--
To view, visit http://gerrit.cloudera.org:8080/10762
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4f9409ca3a519b8b9eb5ca91a3c2a6c7c25e34f
Gerrit-Change-Number: 10762
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Jun 2018 23:37:19 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5873: Check for existence of sync file range()
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has abandoned this change. ( http://gerrit.cloudera.org:8080/10762 )
Change subject: IMPALA-5873: Check for existence of sync_file_range()
......................................................................
Abandoned
Not needed anymore.
--
To view, visit http://gerrit.cloudera.org:8080/10762
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: Ib4f9409ca3a519b8b9eb5ca91a3c2a6c7c25e34f
Gerrit-Change-Number: 10762
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>