Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13986 )
Change subject: IMPALA-8376: directory limits for scratch usage ...................................................................... Patch Set 4: (4 comments) The scope of this expanded a bit since I realised that we didn't support parsing terabyte specifiers yet (I had a test gap there). http://gerrit.cloudera.org:8080/#/c/13986/3/be/src/runtime/tmp-file-mgr-test.cc File be/src/runtime/tmp-file-mgr-test.cc: http://gerrit.cloudera.org:8080/#/c/13986/3/be/src/runtime/tmp-file-mgr-test.cc@785 PS3, Line 785: GetValue()); > nit: file_group_2? Done http://gerrit.cloudera.org:8080/#/c/13986/3/be/src/runtime/tmp-file-mgr-test.cc@805 PS3, Line 805: // Configure various directories with valid formats. I missed verifying the actual values of the limits. In particular, 5tb is not valid - the utility didn't support parsing TB. This seems generally useful so I added it. http://gerrit.cloudera.org:8080/#/c/13986/3/be/src/runtime/tmp-file-mgr-test.cc@834 PS3, Line 834: EXPECT_EQ(4, dirs3.size()); > so this means I can specify the same dir twice, potentially with different That is true, there isn't really anything smart to prevent that. I'm not sure if it's worth trying to do anything special to handle it, we don't want to try to be clever for sure. Maybe I could add a test to verify that nothing crazy happens. http://gerrit.cloudera.org:8080/#/c/13986/3/be/src/runtime/tmp-file-mgr.cc File be/src/runtime/tmp-file-mgr.cc: http://gerrit.cloudera.org:8080/#/c/13986/3/be/src/runtime/tmp-file-mgr.cc@131 PS3, Line 131: (ERROR) << " > I am wondering if we should also default to ignoring the dir in case of lim Yeah that seems reasonable. I don't think the stakes are too high - either are OK behaviours in the face of a misconfiguration, but skipping the directory does contain the potential damage. -- To view, visit http://gerrit.cloudera.org:8080/13986 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I696146a65dbb97f1ba200ae472358ae2db6eb441 Gerrit-Change-Number: 13986 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Abhishek Rawat <[email protected]> Gerrit-Reviewer: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Fri, 02 Aug 2019 23:43:00 +0000 Gerrit-HasComments: Yes
