You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Dan Hecht (Code Review)" <ge...@cloudera.org> on 2018/06/04 18:52:22 UTC

[Impala-ASF-CR] IMPALA-7099: Don't set FILESYSTEM PREFIX for s3

Dan Hecht has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10595


Change subject: IMPALA-7099: Don't set FILESYSTEM_PREFIX for s3
......................................................................

IMPALA-7099: Don't set FILESYSTEM_PREFIX for s3

When S3 support was originally added to Impala, Impala still required
that the defaultFs point to HDFS, and so the S3 tests were originally
run as a "secondary" filesystem (which meant that all paths had to be
fully qualified).

Since then, the restriction on defaultFs has been addressed, and
additionally, the S3 test configuration had been changed to run with S3
as the defaultFs. So, setting FILESYSTEM_PREFIX for S3 is no longer
necessary.

Let's remove that so that S3 tests run in a configuration more similar
to HDFS, which will help prevent them from breaking as often (since
precheckin tests run only in the HDFS configuration).

Incidently, this addresses the problem with
test_unsupported_text_compression on S3 since the problem is with the
handling of fully qualified paths.

Change-Id: Iace5fc9557ff6e7d184573beafea694191424742
---
M bin/impala-config.sh
1 file changed, 0 insertions(+), 1 deletion(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/10595/1
-- 
To view, visit http://gerrit.cloudera.org:8080/10595
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iace5fc9557ff6e7d184573beafea694191424742
Gerrit-Change-Number: 10595
Gerrit-PatchSet: 1
Gerrit-Owner: Dan Hecht <dh...@cloudera.com>

[Impala-ASF-CR] IMPALA-7099: Don't set FILESYSTEM PREFIX for s3

Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10595 )

Change subject: IMPALA-7099: Don't set FILESYSTEM_PREFIX for s3
......................................................................

IMPALA-7099: Don't set FILESYSTEM_PREFIX for s3

When S3 support was originally added to Impala, Impala still required
that the defaultFs point to HDFS, and so the S3 tests were originally
run as a "secondary" filesystem (which meant that all paths had to be
fully qualified).

Since then, the restriction on defaultFs has been addressed, and
additionally, the S3 test configuration had been changed to run with S3
as the defaultFs. So, setting FILESYSTEM_PREFIX for S3 is no longer
necessary.

Let's remove that so that S3 tests run in a configuration more similar
to HDFS, which will help prevent them from breaking as often (since
precheckin tests run only in the HDFS configuration).

Incidently, this addresses the problem with
test_unsupported_text_compression on S3 since the problem is with the
handling of fully qualified paths.

Testing:
- core on S3

Change-Id: Iace5fc9557ff6e7d184573beafea694191424742
Reviewed-on: http://gerrit.cloudera.org:8080/10595
Reviewed-by: Dan Hecht <dh...@cloudera.com>
Tested-by: Dan Hecht <dh...@cloudera.com>
---
M bin/impala-config.sh
1 file changed, 0 insertions(+), 1 deletion(-)

Approvals:
  Dan Hecht: Looks good to me, approved; Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/10595
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Iace5fc9557ff6e7d184573beafea694191424742
Gerrit-Change-Number: 10595
Gerrit-PatchSet: 4
Gerrit-Owner: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-7099: Don't set FILESYSTEM PREFIX for s3

Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Hello Sailesh Mukil, Tim Armstrong, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/10595

to look at the new patch set (#3).

Change subject: IMPALA-7099: Don't set FILESYSTEM_PREFIX for s3
......................................................................

IMPALA-7099: Don't set FILESYSTEM_PREFIX for s3

When S3 support was originally added to Impala, Impala still required
that the defaultFs point to HDFS, and so the S3 tests were originally
run as a "secondary" filesystem (which meant that all paths had to be
fully qualified).

Since then, the restriction on defaultFs has been addressed, and
additionally, the S3 test configuration had been changed to run with S3
as the defaultFs. So, setting FILESYSTEM_PREFIX for S3 is no longer
necessary.

Let's remove that so that S3 tests run in a configuration more similar
to HDFS, which will help prevent them from breaking as often (since
precheckin tests run only in the HDFS configuration).

Incidently, this addresses the problem with
test_unsupported_text_compression on S3 since the problem is with the
handling of fully qualified paths.

Testing:
- core on S3

Change-Id: Iace5fc9557ff6e7d184573beafea694191424742
---
M bin/impala-config.sh
1 file changed, 0 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/10595/3
-- 
To view, visit http://gerrit.cloudera.org:8080/10595
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iace5fc9557ff6e7d184573beafea694191424742
Gerrit-Change-Number: 10595
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-7099: Don't set FILESYSTEM PREFIX for s3

Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/10595 )

Change subject: IMPALA-7099: Don't set FILESYSTEM_PREFIX for s3
......................................................................


Patch Set 3:

Validated using an S3 core build. The standard GVO wouldn't take this path, so it wouldn't help validate this much.


-- 
To view, visit http://gerrit.cloudera.org:8080/10595
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iace5fc9557ff6e7d184573beafea694191424742
Gerrit-Change-Number: 10595
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Jun 2018 00:42:56 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7099: Don't set FILESYSTEM PREFIX for s3

Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/10595 )

Change subject: IMPALA-7099: Don't set FILESYSTEM_PREFIX for s3
......................................................................


Patch Set 3: Code-Review+2

Rebase, trivial merge conflict resolved.


-- 
To view, visit http://gerrit.cloudera.org:8080/10595
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iace5fc9557ff6e7d184573beafea694191424742
Gerrit-Change-Number: 10595
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Jun 2018 00:38:49 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7099: Don't set FILESYSTEM PREFIX for s3

Posted by "Sailesh Mukil (Code Review)" <ge...@cloudera.org>.
Sailesh Mukil has posted comments on this change. ( http://gerrit.cloudera.org:8080/10595 )

Change subject: IMPALA-7099: Don't set FILESYSTEM_PREFIX for s3
......................................................................


Patch Set 2: Code-Review+2

Thanks for doing this! We should've ideally done it a long time back and I missed it.


-- 
To view, visit http://gerrit.cloudera.org:8080/10595
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iace5fc9557ff6e7d184573beafea694191424742
Gerrit-Change-Number: 10595
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Comment-Date: Tue, 05 Jun 2018 16:14:02 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7099: Don't set FILESYSTEM PREFIX for s3

Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/10595 )

Change subject: IMPALA-7099: Don't set FILESYSTEM_PREFIX for s3
......................................................................


Patch Set 3: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/10595
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iace5fc9557ff6e7d184573beafea694191424742
Gerrit-Change-Number: 10595
Gerrit-PatchSet: 3
Gerrit-Owner: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Jun 2018 00:42:08 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7099: Don't set FILESYSTEM PREFIX for s3

Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/10595 )

Change subject: IMPALA-7099: Don't set FILESYSTEM_PREFIX for s3
......................................................................

IMPALA-7099: Don't set FILESYSTEM_PREFIX for s3

When S3 support was originally added to Impala, Impala still required
that the defaultFs point to HDFS, and so the S3 tests were originally
run as a "secondary" filesystem (which meant that all paths had to be
fully qualified).

Since then, the restriction on defaultFs has been addressed, and
additionally, the S3 test configuration had been changed to run with S3
as the defaultFs. So, setting FILESYSTEM_PREFIX for S3 is no longer
necessary.

Let's remove that so that S3 tests run in a configuration more similar
to HDFS, which will help prevent them from breaking as often (since
precheckin tests run only in the HDFS configuration).

Incidently, this addresses the problem with
test_unsupported_text_compression on S3 since the problem is with the
handling of fully qualified paths.

Testing:
- core on S3

Change-Id: Iace5fc9557ff6e7d184573beafea694191424742
---
M bin/impala-config.sh
1 file changed, 0 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/10595/2
-- 
To view, visit http://gerrit.cloudera.org:8080/10595
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iace5fc9557ff6e7d184573beafea694191424742
Gerrit-Change-Number: 10595
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Hecht <dh...@cloudera.com>

[Impala-ASF-CR] IMPALA-7099: Don't set FILESYSTEM PREFIX for s3

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10595 )

Change subject: IMPALA-7099: Don't set FILESYSTEM_PREFIX for s3
......................................................................


Patch Set 2:

Do you want to go ahead and start this merge? It would be good to get this in to reduce the probability of future issues.


-- 
To view, visit http://gerrit.cloudera.org:8080/10595
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iace5fc9557ff6e7d184573beafea694191424742
Gerrit-Change-Number: 10595
Gerrit-PatchSet: 2
Gerrit-Owner: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Jun 2018 00:25:50 +0000
Gerrit-HasComments: No