Todd Lipcon has posted comments on this change. Change subject: Persistent cache support for NVM ......................................................................
Patch Set 11: (4 comments) http://gerrit.cloudera.org:8080/#/c/2593/11/src/kudu/util/nvm_cache.cc File src/kudu/util/nvm_cache.cc: Line 14: // We use this API to implement a cache which treats persistent memory or > I think this comment isn't correct. I would say: Done Line 164: uint8_t* kv_data; // Either pointer to pmem or space for volatile pmem. > This comment doesn't make sense. I wrote it so it's on me :-). I think this Done Line 308: // Wrapper around vmem allocation which injects failures based on a flag. > Is this true? We do have the capability to inject failures but this functio not sure what you mean - all it does if injection isn't enabled is delegate to vmem_malloc, right? Line 463: l.unlock(); > We do a FreeLRUEntries after this loop. right, but this eviction call isnt' actually freeing up any NVM space, so if we failed the first time through, it is going to just keep failing for every subsequent attempt. -- To view, visit http://gerrit.cloudera.org:8080/2593 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id72570ead662670bf42175756a18ae08d7cd0a07 Gerrit-PatchSet: 11 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Sarah Jelinek <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Sarah Jelinek <[email protected]> Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
