Uploaded image for project: 'MariaDB Server'
  1. MariaDB Server
  2. MDEV-35506

commit policy of one-phase-commit even at errored-out binlogging leads to assert

Details

    Description

      Currently execution of
      commit in one phase proceeds to commit by engines when binlog_commit() does not succeed.

      There are two issues with that:
      1. absence of binlog_rollback() or lower-level `binlog_cache_data::reset()` along the following execution of the failing statement eventually will raise an assert on non-empty binlog cache;
      2. engines, including ones that are rollback capable, commit in this particular error situation.

      P.1,2 can be verified with the following mtr script

      source include/have_innodb.inc;
      		 
      source include/have_log_bin.inc;
      		 
       
      		 
      create table ta (a int) engine=aria;
      		 
      create table ti (a int) engine=innodb;
      		 
      delimiter |;
      		 
       
      		 
      create function f_ia()
      		 
      returns integer
      		 
      begin
      		 
        insert into ti set a=1;
      		 
        insert into ta set a=1;
      		 
        return 1;
      		 
      end |
      		 
      delimiter ;|
      		 
       
      		 
      set @@global.gtid_strict_mode=0; set @@session.gtid_seq_no=1;
      		 
      set @@global.gtid_strict_mode=1;
      		 
      --replace_regex /GTID 1-1-[0-9]+/GTID VALUE/
      		 
      --error ER_GTID_STRICT_OUT_OF_ORDER
      		 
      set statement binlog_format=row FOR create table f_x (a int) select f_ia() as a;
      		 
       
      		 
      # --error assert(sql/log.cc:1712(binlog_close_connection))
      		 
      # --disconnect default

      Attachments

        Issue Links

          relates to

          Bug - A problem which impairs or prevents the functions of the product. MDEV-35207 ignored error at binlogging by CREATE-TABLE-SELECT leads to assert

          • Critical - Crashes, loss of data, severe memory leak.
          • Closed

          Activity

            Ascending order - Click to sort in descending order
            Elkin Andrei Elkin created issue -
            Elkin Andrei Elkin made changes -
            Field Original Value New Value
            Elkin Andrei Elkin made changes -
            Affects Version/s 10.6 [ 24028 ]
            Elkin Andrei Elkin made changes -
            Fix Version/s 10.6 [ 24028 ]
            Elkin Andrei Elkin made changes -
            Status Open [ 1 ] In Progress [ 3 ]
            Elkin Andrei Elkin added a comment - - edited

            Howdy Sergei.

            While I offer a patch that tries to undo as much as it's possible I am not really sure that's the way to go. My doubt remains as it's one-phase commit, we may stick to any resolution
            that satisfy the assert of p.1 description - so a binlog_cache_data::reset() could be done instead.
            Anyway my sympathy lies with the patch .

            Elkin Andrei Elkin added a comment - - edited Howdy Sergei. While I offer a patch that tries to undo as much as it's possible I am not really sure that's the way to go. My doubt remains as it's one-phase commit, we may stick to any resolution that satisfy the assert of p.1 description - so a binlog_cache_data::reset() could be done instead. Anyway my sympathy lies with the patch .
            Elkin Andrei Elkin made changes -
            Assignee Andrei Elkin [ elkin ] Sergei Golubchik [ serg ]
            Status In Progress [ 3 ] In Review [ 10002 ]
            serg Sergei Golubchik made changes -
            Priority Major [ 3 ] Critical [ 2 ]
            serg Sergei Golubchik made changes -
            Assignee Sergei Golubchik [ serg ] Andrei Elkin [ elkin ]
            Status In Review [ 10002 ] Stalled [ 10000 ]
            Elkin Andrei Elkin made changes -
            Status Stalled [ 10000 ] In Progress [ 3 ]
            Elkin Andrei Elkin added a comment - - edited

            Sergei, salve.

            I renewed the reviewed branch
            63fa5d717ca...162b8ff486b HEAD -> bb-10.6-MDEV-35506
            An incremental commit addresses MDEV-35207's test which managed to
            slip an Incident event into binlog.
            It is tackled.

            Elkin Andrei Elkin added a comment - - edited Sergei, salve. I renewed the reviewed branch 63fa5d717ca...162b8ff486b HEAD -> bb-10.6-MDEV-35506 An incremental commit addresses MDEV-35207 's test which managed to slip an Incident event into binlog. It is tackled.
            Elkin Andrei Elkin made changes -
            Assignee Andrei Elkin [ elkin ] Sergei Golubchik [ serg ]
            Status In Progress [ 3 ] In Review [ 10002 ]
            Elkin Andrei Elkin made changes -
            Assignee Sergei Golubchik [ serg ] Andrei Elkin [ elkin ]
            Elkin Andrei Elkin added a comment -

            serg, taking it back to myself in order to resolve an MSAN finging in `162b8ff486`.

            Elkin Andrei Elkin added a comment - serg , taking it back to myself in order to resolve an MSAN finging in `162b8ff486`.
            Elkin Andrei Elkin made changes -
            Assignee Andrei Elkin [ elkin ] Sergei Golubchik [ serg ]
            Elkin Andrei Elkin added a comment - - edited

            serg: bb-10.6-MDEV-35506 is updated with your notes addressed. Tests are extended to involve a stored function with side effect of modified non-transactional engines (which resulted in a new bug opened, sadly not easy to fix though hopefully not urgent either).

            Elkin Andrei Elkin added a comment - - edited serg : bb-10.6-MDEV-35506 is updated with your notes addressed. Tests are extended to involve a stored function with side effect of modified non-transactional engines (which resulted in a new bug opened, sadly not easy to fix though hopefully not urgent either).
            serg Sergei Golubchik added a comment -

            86bb79e5d3e0 is ok to push

            serg Sergei Golubchik added a comment - 86bb79e5d3e0 is ok to push
            serg Sergei Golubchik made changes -
            Assignee Sergei Golubchik [ serg ] Andrei Elkin [ elkin ]
            Status In Review [ 10002 ] Stalled [ 10000 ]
            Elkin Andrei Elkin made changes -
            Status Stalled [ 10000 ] In Progress [ 3 ]
            Elkin Andrei Elkin made changes -
            Component/s Replication [ 10100 ]
            Component/s Server [ 13907 ]
            Fix Version/s 10.11.12 [ 29998 ]
            Fix Version/s 11.4.6 [ 29999 ]
            Fix Version/s 11.8.2 [ 30001 ]
            Fix Version/s 10.6 [ 24028 ]
            Resolution Fixed [ 1 ]
            Status In Progress [ 3 ] Closed [ 6 ]

            People

              Elkin Andrei Elkin
              Elkin Andrei Elkin
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Git Integration

                  Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.