Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15313 )
Change subject: KUDU-3063: Set a ratio to reserve some nvm memory ...................................................................... Patch Set 3: (5 comments) http://gerrit.cloudera.org:8080/#/c/15313/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/15313/1//COMMIT_MSG@8 PS1, Line 8: Could you add more information to provide more context here? I.e., describing the essence of the issue: the fragmentation. Also, I think adding a reference to https://github.com/memkind/memkind/blob/master/test/fragmentation_benchmark_pmem.cpp is useful as well. http://gerrit.cloudera.org:8080/#/c/15313/1/src/kudu/util/nvm_cache.cc File src/kudu/util/nvm_cache.cc: http://gerrit.cloudera.org:8080/#/c/15313/1/src/kudu/util/nvm_cache.cc@76 PS1, Line 76: 1.2 > There's a micro benchmark under memkind, https://github.com/memkind/memkind Thank you for the information. Hopefully, 0.7 (or 10/7 with multiplying memkind_malloc_usable_size upon allocations) is indeed the worst case. http://gerrit.cloudera.org:8080/#/c/15313/1/src/kudu/util/nvm_cache.cc@80 PS1, Line 80: > Is that ok to validate it before memkind_create_pmem and also log the reduc I think with the approach of multiplying memkind_malloc_usable_size, it's enough to make sure the multiplication factor is greater that 1: if it's less, return an error. If it's less than 5/4, issue a warning about memkind fragmentation issues. http://gerrit.cloudera.org:8080/#/c/15313/3/src/kudu/util/nvm_cache.cc File src/kudu/util/nvm_cache.cc: http://gerrit.cloudera.org:8080/#/c/15313/3/src/kudu/util/nvm_cache.cc@77 PS3, Line 77: The usage is equal to block " : "cache capacity divided by the ratio It seems this has changed in PS3 (compared with PS2) once the semantics of the --nvm_cache_usage_ration has changed. http://gerrit.cloudera.org:8080/#/c/15313/3/src/kudu/util/nvm_cache.cc@714 PS3, Line 714: if (ratio < 1.0) { : LOG(WARNING) << "Confiture NVM usage ratio " << ratio << " is lower than minimum value 1.0. " : << "Raise --nvm_cache_usage_ratio "; : } else { I think it's safer to prohibit setting ratio less than 1 at all (with new semantics for that parameter). Most appropriate place to do so in a flag validator for the --nvm_cache_usage_ratio flag, so the issue is reported upfront when the process is starting. For a reference/example, you can take a look at https://github.com/apache/kudu/blob/master/src/kudu/util/process_memory.cc#L108-L120 -- To view, visit http://gerrit.cloudera.org:8080/15313 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iceb1931f5addbf76774854ca1613b6a085b577e3 Gerrit-Change-Number: 15313 Gerrit-PatchSet: 3 Gerrit-Owner: ye yuqiang <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: ye yuqiang <[email protected]> Gerrit-Comment-Date: Thu, 05 Mar 2020 20:09:06 +0000 Gerrit-HasComments: Yes
