From: Sergey Vojtovich Date: March 23 2009 12:37pm Subject: Re: Patch for MYISAM to mmap keys List-Archive: http://lists.mysql.com/internals/36464 Message-Id: <20090323123725.GA4706@july.mysql.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7BIT Hi Mark, please accept my congratulations with signing SCA and excuses for not responding for such a long time. Your contribution looks like a great feature and I believe it'll be really useful in some scenarios. I was assigned to review this patch and expect it'll take a few review cycles until we will end up with a "good to push" patch. Ok, now a few issues that were identified during quick overview of the patch... First of all we need to put your patch in conformance with MySQL Internals Coding Guidelines, as described at: http://forge.mysql.com/wiki/MySQL_Internals_Coding_Guidelines The top three rules are: - for identation use two spaces (tabs are disallowed); - open braces should be on a separate line (except for "switch"); - for assigning values to variables we use var= val (this makes easier to search for assignments using grep tool); The second point is that for most system calls we use mysys wrappers. These wrappers mainly guarantee portability. In our case we should use these wrappers for mapping/unmapping index file. See also mysys/my_mmap.c. We need to extend the patch, so this feature can be easily disabled/enabled. I'd suggest to add myisam_use_mmap_for_index variable for this purpose (for an example see how myisam_use_mmap is implemented). That's all for now. Please let me know if you have any concerns/questions. Thanks, Sergey -- Sergey Vojtovich MySQL AB, Software Engineer Izhevsk, Russia, www.mysql.com