Alex Behm has posted comments on this change.

Change subject: Consolidate test and cluster logs under a single directory.
......................................................................


Patch Set 2:

(4 comments)

Michael, to answer your questions:
1. I tested this patch on a full data load in Jenkins. The full data load also 
runs the tests in core.
2. You are correct. There is one change we need to make to Impala-aux due to 
renaming cluster_logs.
3. The informal plan for merging this patch looks something like this:
- once I have +2, do another full data load + exhaustive run and check the logs
- post a CR for fixing Impala-aux, to be merged roughly at the same time as 
this patch
- GVM this patch
- once the patch is in, immediately fix all Jenkins jobs to pick up the new 
artifacts in logs (could also do that before merging this)
- if anything breaks, fight the fire!

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

Line 14: The new structure is as follows:
> This structure explanation is really useful. Is there a way to capture this
I added a LOGS.md in IMPALA_HOME. Happy to change the filename or accommodate 
other suggestions.


http://gerrit.cloudera.org:8080/#/c/2456/2/bin/impala-config.sh
File bin/impala-config.sh:

Line 151: rm -rf ${IMPALA_HOME}/Testing
> Ahh yes! Let me play around some more with the BE tests to make sure we rea
I checked all tests, and modified a few to make sure they do the proper logging.

The remaining test fixes are already covered by Skye's patch here:
http://gerrit.cloudera.org:8080/2471


http://gerrit.cloudera.org:8080/#/c/2456/2/bin/load-data.py
File bin/load-data.py:

Line 260:     assert os.path.isdir(os.path.join(SQL_OUTPUT_DIR, dataset)), 
("Data loading files "
> Could the message also include some hint to the caller as to how to fix the
Good point. I modified the message to hopefully give more clarify.


http://gerrit.cloudera.org:8080/#/c/2456/2/bin/start-impala-cluster.py
File bin/start-impala-cluster.py:

Line 54: parser.add_option("--log_dir", dest="log_dir", 
default=os.environ['IMPALA_CLUSTER_LOGS_DIR'],
> Should this script check for the presence of log_dir now that it doesn't de
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ief1f58f3320ec39d31b3c6bc6ef87f58ff7dfdfa
Gerrit-PatchSet: 2
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Alex Behm <[email protected]>
Gerrit-Reviewer: Alex Behm <[email protected]>
Gerrit-Reviewer: Michael Brown <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-HasComments: Yes

Reply via email to