You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Yida Wu (Code Review)" <ge...@cloudera.org> on 2021/02/28 03:29:23 UTC

[Impala-ASF-CR] IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build 

Yida Wu has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17136


Change subject: IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build
......................................................................

IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build

In the option of scratch_dirs, it uses absolute paths, doesn't consider
the setting of the default filesystem, however, when passing to
AssignQueue(), it does consider the default filesystem. For example, if
scratch_dirs is set as "/tmp", the scratch dir is supposed to be in the
local filesystem, no matter what default filesystem is set. But in the
AssignQueue(), by default it considers the default filesystem, so
the "/tmp" is treated as "s3a://xxx/tmp" if a s3 path is set as the
default fs. To fix this, the solution is to add a variable to ignore
the default fs for all of the scratch directories when parsing the
paths.

Tests:
Set the default fs as a s3 path, reran
TestDiskSpillConfigurations::test_disk_spill_encryption_disabled
and TestQueryRetries::test_query_retry_reaches_spool_limit.

Change-Id: Ic07945abe65d90235aa8dea92dd3c3821a4f1f53
---
M be/src/runtime/io/disk-io-mgr.cc
M be/src/runtime/io/disk-io-mgr.h
M be/src/runtime/io/scan-range.cc
M be/src/runtime/tmp-file-mgr.cc
4 files changed, 24 insertions(+), 16 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic07945abe65d90235aa8dea92dd3c3821a4f1f53
Gerrit-Change-Number: 17136
Gerrit-PatchSet: 1
Gerrit-Owner: Yida Wu <wy...@gmail.com>

[Impala-ASF-CR] IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build

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

Change subject: IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build
......................................................................

IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build

For start option “scratch_dirs”, it only considers local filesystem as
the default filesystem, regardless of the setting of DefaultFS(for a
remote scratch dir, it needs to explicitly set it with the remote fs
prefix). However, the function AssignQueue() would assign the queue
based on not only the path string but also the default filesystem
setting. For example, if scratch_dirs is set as "/tmp", the scratch dir
is supposed to be in the local filesystem, but the AssignQueue() would
consider it as "s3a://xxx/tmp" if a s3 path is set as the default fs.
To fix this, the solution is to add a bool variable to AssignQueue() to
decide whether or not to check the default fs setting when parsing the
file path. For all of the scratch dirs, AssignQueue() won’t check the
default fs.

Tests:
Set the default fs as a s3 path, reran
TestDiskSpillConfigurations::test_disk_spill_encryption_disabled
and TestQueryRetries::test_query_retry_reaches_spool_limit.

Change-Id: Ic07945abe65d90235aa8dea92dd3c3821a4f1f53
---
M be/src/runtime/io/disk-io-mgr.cc
M be/src/runtime/io/disk-io-mgr.h
M be/src/runtime/io/scan-range.cc
M be/src/runtime/tmp-file-mgr.cc
4 files changed, 24 insertions(+), 16 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic07945abe65d90235aa8dea92dd3c3821a4f1f53
Gerrit-Change-Number: 17136
Gerrit-PatchSet: 2
Gerrit-Owner: Yida Wu <wy...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17136 )

Change subject: IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build
......................................................................


Patch Set 6: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/6934/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic07945abe65d90235aa8dea92dd3c3821a4f1f53
Gerrit-Change-Number: 17136
Gerrit-PatchSet: 6
Gerrit-Owner: Yida Wu <wy...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Wed, 03 Mar 2021 06:05:16 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build

Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/17136 )

Change subject: IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build
......................................................................


Patch Set 4: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic07945abe65d90235aa8dea92dd3c3821a4f1f53
Gerrit-Change-Number: 17136
Gerrit-PatchSet: 4
Gerrit-Owner: Yida Wu <wy...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 02 Mar 2021 17:58:22 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17136 )

Change subject: IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build
......................................................................


Patch Set 6: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic07945abe65d90235aa8dea92dd3c3821a4f1f53
Gerrit-Change-Number: 17136
Gerrit-PatchSet: 6
Gerrit-Owner: Yida Wu <wy...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Wed, 03 Mar 2021 22:45:48 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build

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

Change subject: IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17136/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/17136/2//COMMIT_MSG@23
PS2, Line 23: Set the default fs as a s3 path, reran
Could you add an unit-test?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic07945abe65d90235aa8dea92dd3c3821a4f1f53
Gerrit-Change-Number: 17136
Gerrit-PatchSet: 2
Gerrit-Owner: Yida Wu <wy...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Mar 2021 00:22:47 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17136 )

Change subject: IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/8266/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic07945abe65d90235aa8dea92dd3c3821a4f1f53
Gerrit-Change-Number: 17136
Gerrit-PatchSet: 2
Gerrit-Owner: Yida Wu <wy...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Mon, 01 Mar 2021 16:14:55 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17136 )

Change subject: IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build
......................................................................


Patch Set 3:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/8278/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic07945abe65d90235aa8dea92dd3c3821a4f1f53
Gerrit-Change-Number: 17136
Gerrit-PatchSet: 3
Gerrit-Owner: Yida Wu <wy...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 02 Mar 2021 02:36:27 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17136 )

Change subject: IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build
......................................................................


Patch Set 5:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/6932/ DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic07945abe65d90235aa8dea92dd3c3821a4f1f53
Gerrit-Change-Number: 17136
Gerrit-PatchSet: 5
Gerrit-Owner: Yida Wu <wy...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 02 Mar 2021 18:29:13 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build

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

Change subject: IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build
......................................................................


Patch Set 3: Code-Review+1

(2 comments)

http://gerrit.cloudera.org:8080/#/c/17136/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/17136/3//COMMIT_MSG@9
PS3, Line 9: “
nit: non-ascii double quote "


http://gerrit.cloudera.org:8080/#/c/17136/3//COMMIT_MSG@19
PS3, Line 19: ’
nit: non-ascii single quote '



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic07945abe65d90235aa8dea92dd3c3821a4f1f53
Gerrit-Change-Number: 17136
Gerrit-PatchSet: 3
Gerrit-Owner: Yida Wu <wy...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 02 Mar 2021 09:13:50 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17136 )

Change subject: IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build
......................................................................


Patch Set 6:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/6934/ DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic07945abe65d90235aa8dea92dd3c3821a4f1f53
Gerrit-Change-Number: 17136
Gerrit-PatchSet: 6
Gerrit-Owner: Yida Wu <wy...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Wed, 03 Mar 2021 00:02:01 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17136 )

Change subject: IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build
......................................................................


Patch Set 6: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic07945abe65d90235aa8dea92dd3c3821a4f1f53
Gerrit-Change-Number: 17136
Gerrit-PatchSet: 6
Gerrit-Owner: Yida Wu <wy...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Wed, 03 Mar 2021 00:02:00 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17136 )

Change subject: IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build
......................................................................


Patch Set 5: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/6932/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic07945abe65d90235aa8dea92dd3c3821a4f1f53
Gerrit-Change-Number: 17136
Gerrit-PatchSet: 5
Gerrit-Owner: Yida Wu <wy...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 02 Mar 2021 22:43:23 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build

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

Change subject: IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/17136/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/17136/2//COMMIT_MSG@23
PS2, Line 23: Set the default fs as a s3 path, reran
> Could you add an unit-test?
Done


http://gerrit.cloudera.org:8080/#/c/17136/2/be/src/runtime/tmp-file-mgr.cc
File be/src/runtime/tmp-file-mgr.cc:

http://gerrit.cloudera.org:8080/#/c/17136/2/be/src/runtime/tmp-file-mgr.cc@663
PS2, Line 663: -1, true, false
> When you have long parameter lists with constants where its not clear what 
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic07945abe65d90235aa8dea92dd3c3821a4f1f53
Gerrit-Change-Number: 17136
Gerrit-PatchSet: 2
Gerrit-Owner: Yida Wu <wy...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 02 Mar 2021 02:18:33 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build

Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/17136 )

Change subject: IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build
......................................................................

IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build

For start option "scratch_dirs", it only considers local filesystem as
the default filesystem, regardless of the setting of DefaultFS(for a
remote scratch dir, it needs to explicitly set it with the remote fs
prefix). However, the function AssignQueue() would assign the queue
based on not only the path string but also the default filesystem
setting. For example, if scratch_dirs is set as "/tmp", the scratch dir
is supposed to be in the local filesystem, but the AssignQueue() would
consider it as "s3a://xxx/tmp" if a s3 path is set as the default fs.
To fix this, the solution is to add a bool variable to AssignQueue() to
decide whether or not to check the default fs setting when parsing the
file path. For all of the scratch dirs, AssignQueue() won't check the
default fs.

Tests:
Added a unit testcase: TmpFileMgrTest::TestSpillingWithRemoteDefaultFS.
Ran and Passed TmpFileMgrTest.

Change-Id: Ic07945abe65d90235aa8dea92dd3c3821a4f1f53
Reviewed-on: http://gerrit.cloudera.org:8080/17136
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/runtime/io/disk-io-mgr.cc
M be/src/runtime/io/disk-io-mgr.h
M be/src/runtime/io/scan-range.cc
M be/src/runtime/test-env.h
M be/src/runtime/tmp-file-mgr-test.cc
M be/src/runtime/tmp-file-mgr.cc
6 files changed, 56 insertions(+), 16 deletions(-)

Approvals:
  Impala Public Jenkins: Looks good to me, approved; Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic07945abe65d90235aa8dea92dd3c3821a4f1f53
Gerrit-Change-Number: 17136
Gerrit-PatchSet: 7
Gerrit-Owner: Yida Wu <wy...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>

[Impala-ASF-CR] IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build

Posted by "Yida Wu (Code Review)" <ge...@cloudera.org>.
Yida Wu has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/17136 )

Change subject: IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build
......................................................................

IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build

For start option “scratch_dirs”, it only considers local filesystem as
the default filesystem, regardless of the setting of DefaultFS(for a
remote scratch dir, it needs to explicitly set it with the remote fs
prefix). However, the function AssignQueue() would assign the queue
based on not only the path string but also the default filesystem
setting. For example, if scratch_dirs is set as "/tmp", the scratch dir
is supposed to be in the local filesystem, but the AssignQueue() would
consider it as "s3a://xxx/tmp" if a s3 path is set as the default fs.
To fix this, the solution is to add a bool variable to AssignQueue() to
decide whether or not to check the default fs setting when parsing the
file path. For all of the scratch dirs, AssignQueue() won’t check the
default fs.

Tests:
Added a unit testcase: TmpFileMgrTest::TestSpillingWithRemoteDefaultFS.
Ran and Passed TmpFileMgrTest.

Change-Id: Ic07945abe65d90235aa8dea92dd3c3821a4f1f53
---
M be/src/runtime/io/disk-io-mgr.cc
M be/src/runtime/io/disk-io-mgr.h
M be/src/runtime/io/scan-range.cc
M be/src/runtime/test-env.h
M be/src/runtime/tmp-file-mgr-test.cc
M be/src/runtime/tmp-file-mgr.cc
6 files changed, 56 insertions(+), 16 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic07945abe65d90235aa8dea92dd3c3821a4f1f53
Gerrit-Change-Number: 17136
Gerrit-PatchSet: 3
Gerrit-Owner: Yida Wu <wy...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17136 )

Change subject: IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build
......................................................................


Patch Set 6:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/6937/ DRY_RUN=true


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic07945abe65d90235aa8dea92dd3c3821a4f1f53
Gerrit-Change-Number: 17136
Gerrit-PatchSet: 6
Gerrit-Owner: Yida Wu <wy...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Wed, 03 Mar 2021 17:02:13 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build

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

Change subject: IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build
......................................................................


Patch Set 6:

The reason of dryrun build 6932/6934 failing could be a flaky test, IMPALA-10559. The symptom is similar and the patch passed preview release exhaustive build before which covered the failed testcases. Also, dryrun build 6933 failed with a similar symptom without the patch. I think we can merge the patch, then come back the investigate the cause of the flaky test.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic07945abe65d90235aa8dea92dd3c3821a4f1f53
Gerrit-Change-Number: 17136
Gerrit-PatchSet: 6
Gerrit-Owner: Yida Wu <wy...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Wed, 03 Mar 2021 17:42:16 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17136 )

Change subject: IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build
......................................................................


Patch Set 5: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic07945abe65d90235aa8dea92dd3c3821a4f1f53
Gerrit-Change-Number: 17136
Gerrit-PatchSet: 5
Gerrit-Owner: Yida Wu <wy...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 02 Mar 2021 18:29:12 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build

Posted by "Yida Wu (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Thomas Tauber-Marshall, Wenzhe Zhou, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build
......................................................................

IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build

For start option "scratch_dirs", it only considers local filesystem as
the default filesystem, regardless of the setting of DefaultFS(for a
remote scratch dir, it needs to explicitly set it with the remote fs
prefix). However, the function AssignQueue() would assign the queue
based on not only the path string but also the default filesystem
setting. For example, if scratch_dirs is set as "/tmp", the scratch dir
is supposed to be in the local filesystem, but the AssignQueue() would
consider it as "s3a://xxx/tmp" if a s3 path is set as the default fs.
To fix this, the solution is to add a bool variable to AssignQueue() to
decide whether or not to check the default fs setting when parsing the
file path. For all of the scratch dirs, AssignQueue() won't check the
default fs.

Tests:
Added a unit testcase: TmpFileMgrTest::TestSpillingWithRemoteDefaultFS.
Ran and Passed TmpFileMgrTest.

Change-Id: Ic07945abe65d90235aa8dea92dd3c3821a4f1f53
---
M be/src/runtime/io/disk-io-mgr.cc
M be/src/runtime/io/disk-io-mgr.h
M be/src/runtime/io/scan-range.cc
M be/src/runtime/test-env.h
M be/src/runtime/tmp-file-mgr-test.cc
M be/src/runtime/tmp-file-mgr.cc
6 files changed, 56 insertions(+), 16 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/36/17136/4
-- 
To view, visit http://gerrit.cloudera.org:8080/17136
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic07945abe65d90235aa8dea92dd3c3821a4f1f53
Gerrit-Change-Number: 17136
Gerrit-PatchSet: 4
Gerrit-Owner: Yida Wu <wy...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>

[Impala-ASF-CR] IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build

Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/17136 )

Change subject: IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17136/2/be/src/runtime/tmp-file-mgr.cc
File be/src/runtime/tmp-file-mgr.cc:

http://gerrit.cloudera.org:8080/#/c/17136/2/be/src/runtime/tmp-file-mgr.cc@663
PS2, Line 663: -1, true, false
When you have long parameter lists with constants where its not clear what they correspond to, you can make it more readable by adding the parameter names in /* */, i.e. this line would become:

return file_group_->io_mgr_->AssignQueue(local_buffer_path_.c_str(), /* disk_id */ -1, /* expected_local */ true, /* check_default_fs */ false);

and you can do the same for the other lines, here and in scan-range.cc, where you're passing constants in as parameters



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic07945abe65d90235aa8dea92dd3c3821a4f1f53
Gerrit-Change-Number: 17136
Gerrit-PatchSet: 2
Gerrit-Owner: Yida Wu <wy...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Mar 2021 00:14:17 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/17136 )

Change subject: IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/8260/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic07945abe65d90235aa8dea92dd3c3821a4f1f53
Gerrit-Change-Number: 17136
Gerrit-PatchSet: 1
Gerrit-Owner: Yida Wu <wy...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Sun, 28 Feb 2021 03:48:25 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build

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

Change subject: IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build
......................................................................


Patch Set 2:

> Patch Set 2:
> 
> (1 comment)

Good to see this! I hit the same issue in GCS when working on https://gerrit.cloudera.org/c/17121

I reproduce it using a test in test_queries.py::TestQueries::test_analytic_fns

 SET default_spillable_buffer_size=8m;
 SET buffer_pool_limit=47m;
 SELECT lag(-180, 13) over (ORDER BY t1.int_col ASC, t2.int_col ASC) AS int_col
 FROM functional_parquet.alltypes t1 CROSS JOIN functional_parquet.alltypes t2 
 LIMIT 10;

This patch fixes my issue. Maybe you can add a test similar to this and only run on S3 builds.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic07945abe65d90235aa8dea92dd3c3821a4f1f53
Gerrit-Change-Number: 17136
Gerrit-PatchSet: 2
Gerrit-Owner: Yida Wu <wy...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Mar 2021 01:01:08 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build

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

Change subject: IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build
......................................................................


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/17136/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/17136/3//COMMIT_MSG@9
PS3, Line 9: “
> nit: non-ascii double quote "
Done


http://gerrit.cloudera.org:8080/#/c/17136/3//COMMIT_MSG@19
PS3, Line 19: ’
> nit: non-ascii single quote '
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic07945abe65d90235aa8dea92dd3c3821a4f1f53
Gerrit-Change-Number: 17136
Gerrit-PatchSet: 3
Gerrit-Owner: Yida Wu <wy...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 02 Mar 2021 14:04:51 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build

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

Change subject: IMPALA-10529: Fix hit DCHECK in DiskIoMgr::AssignQueue in core-s3 build
......................................................................


Patch Set 4: Code-Review+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic07945abe65d90235aa8dea92dd3c3821a4f1f53
Gerrit-Change-Number: 17136
Gerrit-PatchSet: 4
Gerrit-Owner: Yida Wu <wy...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Yida Wu <wy...@gmail.com>
Gerrit-Comment-Date: Tue, 02 Mar 2021 17:48:14 +0000
Gerrit-HasComments: No