Yida Wu has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/19496 )

Change subject: IMPALA-11920: Support spill to HDFS address by service ID
......................................................................


Patch Set 3:

(6 comments)

Thanks for the fix, it looks good, just some questions to understand the change.

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

http://gerrit.cloudera.org:8080/#/c/19496/3//COMMIT_MSG@17
PS3, Line 17: priority
Just to confirm that the priority is removed in the document for the remote 
scratch directory, however, can still be parsed in the code.


http://gerrit.cloudera.org:8080/#/c/19496/3/be/src/runtime/tmp-file-mgr-test.cc
File be/src/runtime/tmp-file-mgr-test.cc:

http://gerrit.cloudera.org:8080/#/c/19496/3/be/src/runtime/tmp-file-mgr-test.cc@2240
PS3, Line 2240: hdfs
Maybe it is not related, but when I test with a space behind the url like 
--impalad_args='--scratch_dirs="hdfs://localhost:8020/ , /tmp"', it will try to 
create a path on " hdfs://localhost:8020/ /impala-scratch", could you please 
take a look on the rtrim of the path and add a testcase for it if it is an 
issue?


http://gerrit.cloudera.org:8080/#/c/19496/3/docs/topics/impala_disk_space.xml
File docs/topics/impala_disk_space.xml:

http://gerrit.cloudera.org:8080/#/c/19496/3/docs/topics/impala_disk_space.xml@298
PS3, Line 298: /dir1::0
The alignment looks weird compared to the previous version, does this part show 
normally?


http://gerrit.cloudera.org:8080/#/c/19496/3/docs/topics/impala_disk_space.xml@303
PS3, Line 303: /dir1:200GB
             : /dir1:200GB:
Same as above


http://gerrit.cloudera.org:8080/#/c/19496/3/docs/topics/impala_disk_space.xml@486
PS3, Line 486: ozone1
Just a question that what is the difference between Ozone service identifier 
and Ozone Manager? Is it no difference for the Impala to parse the Ozone 
scratch path?


http://gerrit.cloudera.org:8080/#/c/19496/3/docs/topics/impala_disk_space.xml@488
PS3, Line 488:
nit. looks good to have a comma here



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie069cba211df85fe90d174900b20a26fcc1f7167
Gerrit-Change-Number: 19496
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Smith <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Michael Smith <[email protected]>
Gerrit-Reviewer: Shajini Thayasingh <[email protected]>
Gerrit-Reviewer: Yida Wu <[email protected]>
Gerrit-Comment-Date: Wed, 15 Feb 2023 10:53:25 +0000
Gerrit-HasComments: Yes

Reply via email to