Details

    • Type: Bug
    • Status: Closed
    • Priority: Blocker
    • Resolution: Fixed
    • Affects Version/s: 5.5.24
    • Fix Version/s: 5.5.25
    • Component/s: None
    • Labels:
      None

      Description

      I discovered a serious problem in 5.5 related to fsync (5.3 and below appears
      not affected).

      By default, we do not run any fsync at all. This is because the
      --debug-no-sync option is incorrectly enabled by default.

      This problem was introduced here:

      revid:sergii@pisem.net-20111212225840-8aotf02a1alw1gta

      • {"sync_sys", 0, - "Enable system sync calls. Disable only when running tests or debugging!", - &opt_sync, &opt_sync, 0, GET_BOOL, NO_ARG, 1, 0, 0, 0, 0, 0}

        ,
        +

        {"debug-no-sync", 0, + "Disables system sync calls. Only for running tests or debugging!", + &my_disable_sync, &my_disable_sync, 0, GET_BOOL, NO_ARG, 1, 0, 0, 0, 0, 0}

        ,

      The option was renamed and the meaning was reversed - but the default was left
      unchanged, effectively reversing the default.

      We need to fix this ASAP, as it causes data loss / silent corruption when the
      host machine crashes.

      There is an additional problem that even with --debug-no-sync=0, we are using
      fsync(), not fdatasync() as we should. There is code to use fdatasync(), but
      apparently availability of fdatasync() is not correctly detected by
      cmake. This is also a quite serious issue; one one machine I tested with a
      good RAID controller, fdatasync() is 2.5 times faster than fsync().

      Work-around: pass --debug-no-sync=0 in config or on command line (but this will not fix fdatasync() performance problem).

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            knielsen Kristian Nielsen added a comment -

            This patch seems to fix it:

            === modified file 'config.h.cmake'
            — config.h.cmake 2012-04-19 02:02:28 +0000
            +++ config.h.cmake 2012-06-08 08:49:30 +0000
            @@ -150,6 +150,7 @@
            #cmakedefine HAVE_FCNTL 1
            #cmakedefine HAVE_FCONVERT 1
            #cmakedefine HAVE_FDATASYNC 1
            +#cmakedefine HAVE_DECL_FDATASYNC 1
            #cmakedefine HAVE_FESETROUND 1
            #cmakedefine HAVE_FINITE 1
            #cmakedefine HAVE_FP_EXCEPT 1

            === modified file 'sql/mysqld.cc'
            — sql/mysqld.cc 2012-05-21 18:54:41 +0000
            +++ sql/mysqld.cc 2012-06-08 08:49:06 +0000
            @@ -6161,7 +6161,7 @@ struct my_option my_long_options[]=
            #endif

            {"debug-no-sync", 0, "Disables system sync calls. Only for running tests or debugging!", - &my_disable_sync, &my_disable_sync, 0, GET_BOOL, NO_ARG, 1, 0, 0, 0, 0, 0}

            ,
            + &my_disable_sync, &my_disable_sync, 0, GET_BOOL, NO_ARG, 0, 0, 0, 0, 0, 0},
            #ifdef HAVE_REPLICATION
            {"debug-sporadic-binlog-dump-fail", 0,
            "Option used by mysql-test for debugging and testing of replication.",

            Show
            knielsen Kristian Nielsen added a comment - This patch seems to fix it: === modified file 'config.h.cmake' — config.h.cmake 2012-04-19 02:02:28 +0000 +++ config.h.cmake 2012-06-08 08:49:30 +0000 @@ -150,6 +150,7 @@ #cmakedefine HAVE_FCNTL 1 #cmakedefine HAVE_FCONVERT 1 #cmakedefine HAVE_FDATASYNC 1 +#cmakedefine HAVE_DECL_FDATASYNC 1 #cmakedefine HAVE_FESETROUND 1 #cmakedefine HAVE_FINITE 1 #cmakedefine HAVE_FP_EXCEPT 1 === modified file 'sql/mysqld.cc' — sql/mysqld.cc 2012-05-21 18:54:41 +0000 +++ sql/mysqld.cc 2012-06-08 08:49:06 +0000 @@ -6161,7 +6161,7 @@ struct my_option my_long_options[]= #endif {"debug-no-sync", 0, "Disables system sync calls. Only for running tests or debugging!", - &my_disable_sync, &my_disable_sync, 0, GET_BOOL, NO_ARG, 1, 0, 0, 0, 0, 0} , + &my_disable_sync, &my_disable_sync, 0, GET_BOOL, NO_ARG, 0, 0, 0, 0, 0, 0}, #ifdef HAVE_REPLICATION {"debug-sporadic-binlog-dump-fail", 0, "Option used by mysql-test for debugging and testing of replication.",
            Hide
            knielsen Kristian Nielsen added a comment -

            Pushed to lp:maria/5.5

            Show
            knielsen Kristian Nielsen added a comment - Pushed to lp:maria/5.5

              People

              • Assignee:
                knielsen Kristian Nielsen
                Reporter:
                knielsen Kristian Nielsen
              • Votes:
                0 Vote for this issue
                Watchers:
                1 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:

                  Time Tracking

                  Estimated:
                  Original Estimate - Not Specified
                  Not Specified
                  Remaining:
                  Remaining Estimate - 0 minutes
                  0m
                  Logged:
                  Time Spent - 3 hours
                  3h