Code review comment for lp:~laurynas-biveinis/percona-server/atomic-fio-5.5

Revision history for this message
Alexey Kopytov (akopytov) wrote :

Laurynas,

  - fil_extend_space_to_desired_size() will not work correctly for a
    multi-node ibdata1. The existing code extends the last node, but
    takes the size of other nodes into account by calculating offsets
    as via (start_page_no - file_start_page_no).

    But the new srv_use_posix_fallocate code path just extends the last
    node to the total desired size regardless of other node sizes,
    i.e. overallocate space by always assuming a single-node tablespace.

Minor things (since needs recommitting anyway):

   - spurious comment change on lines 324-247
   - incorrect function calls formatting ("func (...)"):

+ if (ioctl (file, DFS_IOCTL_ATOMIC_WRITE_SET, &atomic_option)) {
+ os_file_handle_error_no_exit (name, "ioctl");
+ os_file_handle_error_no_exit (name, "posix_fallocate");

   - the following doesn't really have any effect, but the convention
     there is to assign variables to their default values, which is
     FALSE for innobase_use_atomic_writes:

+static my_bool innobase_use_atomic_writes = TRUE;

     which is also inconsistent with:

+UNIV_INTERN ibool srv_use_atomic_writes = FALSE;

   - and wrong comment formatting:

+ /* Due to a bug in directFS, using atomics needs
+ * posix_fallocate to extend the file
+ * pwrite() past end of the file won't work
+ */

review: Needs Fixing

« Back to merge proposal