Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15737 )

Change subject: IMPALA-9653: Impala shouldn't create/remove staging directory 
during transactional INSERTs
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/15737/1/be/src/runtime/coordinator.cc
File be/src/runtime/coordinator.cc:

http://gerrit.cloudera.org:8080/#/c/15737/1/be/src/runtime/coordinator.cc@760
PS1, Line 760:     return return_status;
> I am slightly worried about the early delete in a finalize function, as som
I agree, done.


http://gerrit.cloudera.org:8080/#/c/15737/1/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:

http://gerrit.cloudera.org:8080/#/c/15737/1/fe/src/main/java/org/apache/impala/service/Frontend.java@1703
PS1, Line 1703:         finalizeParams.setStaging_dir(
              :             hdfsTable.getHdfsBaseDir() + 
"/_impala_insert_staging");
> I wonder whether this thrift is meaningful at all - we have the same string
It doesn't control where Impala puts the files, but it controls which directory 
Impala deletes (in coordinator.cc).

The two being the same is only enforced by a test in 
test_insert_behaviour.py::test_insert_removes_staging_files.

I agree that it'd be nicer if both were controlled here, but I don't want to 
complicate this small fix.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6a3a502fb3b3cfe7a68323d3b0145e5fb149460
Gerrit-Change-Number: 15737
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]>
Gerrit-Comment-Date: Thu, 16 Apr 2020 11:56:52 +0000
Gerrit-HasComments: Yes

Reply via email to