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

Change subject: IMPALA-11730: Add support for spilling to Ozone
......................................................................


Patch Set 7:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/19251/7//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19251/7//COMMIT_MSG@10
PS7, Line 10: ofs://localhost:9862/impala/tmp
I assume the format for an 'ofs' scheme scratch space should be the same as 
'hdfs', could you also describe the general format of 'ofs' scratch space? For 
example showing the possible options and which is mandate or optional


http://gerrit.cloudera.org:8080/#/c/19251/7//COMMIT_MSG@15
PS7, Line 15: .
Would be good to mention the testcases that have been passed.
By the way, I have seen several failures in DiskIoMgrTest (unit test) when 
setting the target system to ozone, it doesn't happen when the target system is 
hdfs for this patch, not sure whether it is an issue.


http://gerrit.cloudera.org:8080/#/c/19251/7//COMMIT_MSG@17
PS7, Line 17: Refactors TmpDir to remove extraneous variables and functions. 
Each
            : implementation is expected to handle its own token parsing.
Could you also add ofs to the parsing unit test 
TestDirectoryLimitParsingRemotePath?https://github.com/apache/impala/blob/b17446818c548487fa5d71c678088ed35eed5bc8/be/src/runtime/tmp-file-mgr-test.cc#L1086


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

http://gerrit.cloudera.org:8080/#/c/19251/7/be/src/runtime/tmp-file-mgr.cc@693
PS7, Line 693: The priority
nit. a typo from the old code, would be good to fix.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5837c30357363f727ca832fb94169f2474fb4f6f
Gerrit-Change-Number: 19251
Gerrit-PatchSet: 7
Gerrit-Owner: Michael Smith <[email protected]>
Gerrit-Reviewer: Abhishek Rawat <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Michael Smith <[email protected]>
Gerrit-Reviewer: Yida Wu <[email protected]>
Gerrit-Comment-Date: Thu, 01 Dec 2022 17:17:55 +0000
Gerrit-HasComments: Yes

Reply via email to