Matthew Jacobs has posted comments on this change. Change subject: IMPALA-5240: Allow config of number of disk I/O threads per disk type ......................................................................
Patch Set 7: (12 comments) http://gerrit.cloudera.org:8080/#/c/7232/7//COMMIT_MSG Commit Message: PS7, Line 24: TODO (Request comments on this additional change): remove TODO line and update the message below to indicate this all works, the only future work is to consider changing the BE tests to actually have files on different disks, but that's basically unrelated to your change. http://gerrit.cloudera.org:8080/#/c/7232/7/be/src/runtime/disk-io-mgr-stress.cc File be/src/runtime/disk-io-mgr-stress.cc: Line 75: MIN_READ_BUFFER_SIZE, MAX_READ_BUFFER_SIZE)); nit 4 spaces tab http://gerrit.cloudera.org:8080/#/c/7232/7/be/src/runtime/disk-io-mgr.cc File be/src/runtime/disk-io-mgr.cc: Line 48: " disk. Has priority over num_threads_per_disk."); It'd be good to indicate what this defaults to if this and num_threads_per_disk isn't set. Line 51: DEFINE_int32(num_io_threads_per_solid_state_disk, 0, "Number of I/O threads per solid" same Line 262: static inline int GetFirstPositiveVal(const int first_val, const int second_val, 1 line comment why this exists PS7, Line 273: FLASH inconsistent FLASH vs ssd naming PS7, Line 284: && this should be || PS7, Line 375: e add space http://gerrit.cloudera.org:8080/#/c/7232/7/be/src/util/disk-info.h File be/src/util/disk-info.h: Line 56: add a comment that this returns true if the disk w/ disk_id exists and is a rotational disk, is false otherwise. PS7, Line 59: //TODO: temporarily removed DCHECK due to an issue tracked in IMPALA-5574, put it back : // after its resolved remove, I don't think this needs to be temporary. If someone wants to check the number of disks they can use num_disks(). PS7, Line 64: nit remove whitespace PS7, Line 80: remove trailing whitespace -- To view, visit http://gerrit.cloudera.org:8080/7232 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I094aff3747104104fe0465d73dcdbef5d9892f7c Gerrit-PatchSet: 7 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
