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

Remove backup_fix_ddl() during backup

Details

    • Bug
    • Status: Open (View Workflow)
    • Major
    • Resolution: Unresolved
    • 10.2.21, 10.3.12, 10.4(EOL)
    • 10.4(EOL)
    • mariabackup
    • None

    Description

      If MLOG_FILE_* redo log records works as expected during prepare phase then there is no need for extra tracking of ddl. Sample is that MLOG_FILE_RENAME2 didn't work as expected in prepare phase because mariabackup stores file name as "test/t1.ibd" but redo log stores "./test/t1.ibd". So fil_op_replay_rename() does string matching and fails.

      Attachments

        Issue Links

          Activity

            We should also change mariabackup --prepare so that fil_name_parse() will actually delete files when processing MLOG_FILE_DELETE records, similar to how it renames files when processing MLOG_FILE_RENAME2. (If a file by the name exists, and the tablespace ID inside it matches, then delete the file.)

            During normal crash recovery, MLOG_FILE_DELETE is informational and should not need to be applied, but I do not think that deleting files during recovery could hurt correctness.

            marko Marko Mäkelä added a comment - We should also change mariabackup --prepare so that fil_name_parse() will actually delete files when processing MLOG_FILE_DELETE records, similar to how it renames files when processing MLOG_FILE_RENAME2 . (If a file by the name exists, and the tablespace ID inside it matches, then delete the file.) During normal crash recovery, MLOG_FILE_DELETE is informational and should not need to be applied, but I do not think that deleting files during recovery could hurt correctness.

            I initially thought that this issue would block MDEV-18194. In fact, it does not; the problems found during the testing can be fixed in other ways. One of the failures that I wrongly suspected to be fixed by this one was filed as MDEV-18415.
            I think that fixing the InnoDB redo log based handling of DDL during backup (this ticket) has a lower priority than MDEV-18194 and MDEV-18415.

            marko Marko Mäkelä added a comment - I initially thought that this issue would block MDEV-18194 . In fact, it does not; the problems found during the testing can be fixed in other ways. One of the failures that I wrongly suspected to be fixed by this one was filed as MDEV-18415 . I think that fixing the InnoDB redo log based handling of DDL during backup (this ticket) has a lower priority than MDEV-18194 and MDEV-18415 .
            vlad.lesin Vladislav Lesin added a comment - - edited

            Some comments, how it could be implemented in 10.6+.

            The general idea is to move backup_fix_ddl() functionality from mariabackup to recv_init_crash_recovery_spaces().

            So how does innodb ddl fixing work in mariabackup:
            1) ddl_tracker.id_to_name is filled when "create" or "rename" redo log records are parsed during backup;
            2) ddl_tracker.drops contains space id's which were parsed from "delete" redo log records during backup.

            Then all spaces which name from ddl_tracker.id_to_name do not match their name in backup directory are treated as "renamed". The rest of the spaces from ddl_tracker.id_to_name which id is not in ddl_tracker.drops are treated as "created", the rest of the spaces are treated as "deleted". On --prepare ddl's are processed with the following order: "deleted", "renamed", "created".

            In the case of InnoDB recovery the role of ddl_tracker.id_to_name and ddl_tracker.drops plays recv_spaces, which contains space_id->file_name_t pairs, and file_name_t has status "normal", "deleted" or "missed", and is filled when mfile_type_t log records are parsed. renamed_spaces is introduced in MDEV-24184 fix. So all spaces in recv_spaces which are not in renamed_spaces and have status "normal" can be treated as created. We can do this even simpler filling separate containers for created and dropped spaces during log file parsing similar to renamed_spaces. So we if srv_operation == SRV_OPERATION_RESTORE, we can process ddl's parsed from redo log. We need to delete spaces marked as "deleted", rename "renamed" spaces, and create spaces with the corresponding id's for "created" spaces before any log records are applied, and recv_init_crash_recovery_spaces() suits for this purpose.

            See also this comment https://jira.mariadb.org/browse/MDEV-24184?focusedCommentId=182121&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-182121.

            vlad.lesin Vladislav Lesin added a comment - - edited Some comments, how it could be implemented in 10.6+. The general idea is to move backup_fix_ddl() functionality from mariabackup to recv_init_crash_recovery_spaces(). So how does innodb ddl fixing work in mariabackup: 1) ddl_tracker.id_to_name is filled when "create" or "rename" redo log records are parsed during backup; 2) ddl_tracker.drops contains space id's which were parsed from "delete" redo log records during backup. Then all spaces which name from ddl_tracker.id_to_name do not match their name in backup directory are treated as "renamed". The rest of the spaces from ddl_tracker.id_to_name which id is not in ddl_tracker.drops are treated as "created", the rest of the spaces are treated as "deleted". On --prepare ddl's are processed with the following order: "deleted", "renamed", "created". In the case of InnoDB recovery the role of ddl_tracker.id_to_name and ddl_tracker.drops plays recv_spaces, which contains space_id->file_name_t pairs, and file_name_t has status "normal", "deleted" or "missed", and is filled when mfile_type_t log records are parsed. renamed_spaces is introduced in MDEV-24184 fix. So all spaces in recv_spaces which are not in renamed_spaces and have status "normal" can be treated as created. We can do this even simpler filling separate containers for created and dropped spaces during log file parsing similar to renamed_spaces. So we if srv_operation == SRV_OPERATION_RESTORE, we can process ddl's parsed from redo log. We need to delete spaces marked as "deleted", rename "renamed" spaces, and create spaces with the corresponding id's for "created" spaces before any log records are applied, and recv_init_crash_recovery_spaces() suits for this purpose. See also this comment https://jira.mariadb.org/browse/MDEV-24184?focusedCommentId=182121&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-182121 .
            vlad.lesin Vladislav Lesin added a comment - - edited

            I have doubts about the necessity to implement this task at all.

            Because in fact we need just move some functionality from mariabackup to innodb. backup_fix_ddl() along with .new, .ren, .del files processing does the same thing what we are going to implement, as it is not completely implemented, see my comment above) in InnoDB recovery code. Yes, it would be great to have innodb recovery and backup restore code in one place, but at the moment it does not have any benefits except code nicety. So this is just refactoring.

            Maybe server-side backup(MDEV-14992) would make the implementation of this task more reasonable.

            vlad.lesin Vladislav Lesin added a comment - - edited I have doubts about the necessity to implement this task at all. Because in fact we need just move some functionality from mariabackup to innodb. backup_fix_ddl() along with .new, .ren, .del files processing does the same thing what we are going to implement, as it is not completely implemented, see my comment above) in InnoDB recovery code. Yes, it would be great to have innodb recovery and backup restore code in one place, but at the moment it does not have any benefits except code nicety. So this is just refactoring. Maybe server-side backup( MDEV-14992 ) would make the implementation of this task more reasonable.

            And BTW, why this is "Bug"? It should "Task".

            vlad.lesin Vladislav Lesin added a comment - And BTW, why this is "Bug"? It should "Task".

            10.6-remove-ddl-fixing.diff - is my try to remove "rename" processing from mariabackup, it can be useful for the future work. The patch solves the problem of circular renames, the test case for such renames is implemented in rename_during_backup.test, the test almost passes, it fails when "rename" is combined with "create", what also requires the corresponding fixes for "create" ddl replaying.

            vlad.lesin Vladislav Lesin added a comment - 10.6-remove-ddl-fixing.diff - is my try to remove "rename" processing from mariabackup, it can be useful for the future work. The patch solves the problem of circular renames, the test case for such renames is implemented in rename_during_backup.test, the test almost passes, it fails when "rename" is combined with "create", what also requires the corresponding fixes for "create" ddl replaying.

            People

              thiru Thirunarayanan Balathandayuthapani
              thiru Thirunarayanan Balathandayuthapani
              Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

                Created:
                Updated:

                Git Integration

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