Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11248 )
Change subject: KUDU-2469 pt 1: add an IOContext ...................................................................... Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/11248/1/src/kudu/fs/io_context.h File src/kudu/fs/io_context.h: http://gerrit.cloudera.org:8080/#/c/11248/1/src/kudu/fs/io_context.h@33 PS1, Line 33: // Utility class used to pass strings across many layers, avoiding excessive : // plumbing. > I'd considered going through hte iters, but there are a couple paths to CFi I don't think plumbing into the constructor makes sense; wasn't one of the goals to avoid adding new members to CFileReader? Besides, the ownership model for those classes isn't always straight-forward: IIRC, CFileSet is shared ownership because a new instance is swapped into a DRS during a compaction. Further, I expect IOContext to eventually contain information about the initiator of the IO, in addition to "static" information like the tablet ID. You can imagine implementing interesting QoS or resource management strategies based on the initiator's allowance. You wouldn't be able to account for this stuff if an IOContext was bound to a CFileReader at constructor time. However, you may be able to get away with plumbing an IOContext into the _iterator_ constructors. That means the IOContext can't change between scan chunks (thus no per-RPC state), but maybe that's good enough? Having looked at the second patch, I have an additional concern with the current approach: the loose coupling between the IO "initiator" and the IO "performer" means it's really tough to tell if we're 100% covered on all necessary SCOPED_IO_CONTEXT calls. That loose coupling means we probably can't assert that every IO path actually has an IOContext set, which makes the code more complex. -- To view, visit http://gerrit.cloudera.org:8080/11248 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I645e10a2fda1f2564326b7052e6a0debc5ad738c Gerrit-Change-Number: 11248 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Wong <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Fri, 17 Aug 2018 03:15:00 +0000 Gerrit-HasComments: Yes
