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

Backup may copy unnecessarily much log

Details

    Description

      In mariadb-backup --backup there are multiple mechanisms for ensuring that a sufficient amount of the InnoDB write-ahead log (ib_logfile0) is being copied at the end of the backup.

      There is a function backup_wait_for_lsn(), which will wait for the log_copying_thread() to copy a necessary amount. But, we fail to pass the target LSN to log_copying_thread() itself.

      The log copying and recovery logic was rewritten in MDEV-14425 and some locking in backup was rewritten in MDEV-32932. Therefore, it is not trivial to apply this fix to earlier versions.

      Attachments

        Issue Links

          Activity

            marko Marko Mäkelä created issue -
            marko Marko Mäkelä made changes -
            Field Original Value New Value
            marko Marko Mäkelä made changes -
            marko Marko Mäkelä made changes -
            marko Marko Mäkelä made changes -
            Status Open [ 1 ] In Progress [ 3 ]
            marko Marko Mäkelä made changes -
            Assignee Marko Mäkelä [ marko ] Debarun Banerjee [ JIRAUSER54513 ]
            Status In Progress [ 3 ] In Review [ 10002 ]

            marko Sorry, I delayed review on the patch for long. After checking the details I couldn't agree with the patch and I think the current design is safer and better. I have detailed comments in pull-3370.

            Also, I would strongly recommend such improvement / behaviour change be attempted as a non-functional feature improvement (perhaps only in latest version).

            debarun Debarun Banerjee added a comment - marko Sorry, I delayed review on the patch for long. After checking the details I couldn't agree with the patch and I think the current design is safer and better. I have detailed comments in pull-3370 . Also, I would strongly recommend such improvement / behaviour change be attempted as a non-functional feature improvement (perhaps only in latest version).
            debarun Debarun Banerjee made changes -
            Assignee Debarun Banerjee [ JIRAUSER54513 ] Marko Mäkelä [ marko ]
            Status In Review [ 10002 ] Stalled [ 10000 ]

            Thank you for the review. Based on MDEV-5336, part of my attempted logic change seems to be incorrect, and we really have to copy the ib_logfile0 until the LSN as it is right after BACKUP STAGE BLOCK_COMMIT.

            marko Marko Mäkelä added a comment - Thank you for the review. Based on MDEV-5336 , part of my attempted logic change seems to be incorrect, and we really have to copy the ib_logfile0 until the LSN as it is right after BACKUP STAGE BLOCK_COMMIT .
            marko Marko Mäkelä made changes -
            Status Stalled [ 10000 ] In Progress [ 3 ]

            I have revised this. The logic is now somewhat clearer at least to me. All InnoDB log copying would now take place in the log_copying_thread.

            One surprise to me was that executing any statement (such as SHOW ENGINE INNODB STATUS) could cause a write to the table mysql.general_log. Therefore, we must be sure to copy the file mysql/general_log.CSM before trying to determine the InnoDB log sequence number.

            marko Marko Mäkelä added a comment - I have revised this. The logic is now somewhat clearer at least to me. All InnoDB log copying would now take place in the log_copying_thread . One surprise to me was that executing any statement (such as SHOW ENGINE INNODB STATUS ) could cause a write to the table mysql.general_log . Therefore, we must be sure to copy the file mysql/general_log.CSM before trying to determine the InnoDB log sequence number.
            marko Marko Mäkelä made changes -
            Assignee Marko Mäkelä [ marko ] Debarun Banerjee [ JIRAUSER54513 ]
            Status In Progress [ 3 ] In Review [ 10002 ]

            Thanks marko , the patch looks functionally correct to me and the refactoring is also good. I have some comments for you to consider at code level.

            I still don't completely understand the requirement that we need to copy less redo logs and stay behind. Perhaps the backup could perform faster but user could pay bigger penalty in replication setup. I have tried to explain my P.O.V. in one of the comments.

            debarun Debarun Banerjee added a comment - Thanks marko , the patch looks functionally correct to me and the refactoring is also good. I have some comments for you to consider at code level. I still don't completely understand the requirement that we need to copy less redo logs and stay behind. Perhaps the backup could perform faster but user could pay bigger penalty in replication setup. I have tried to explain my P.O.V. in one of the comments.
            debarun Debarun Banerjee made changes -
            Status In Review [ 10002 ] Stalled [ 10000 ]
            debarun Debarun Banerjee made changes -
            Assignee Debarun Banerjee [ JIRAUSER54513 ] Marko Mäkelä [ marko ]
            mariadb-jira-automation Jira Automation (IT) made changes -
            Zendesk Related Tickets 204878
            Zendesk active tickets 204878
            marko Marko Mäkelä made changes -
            Assignee Marko Mäkelä [ marko ] Debarun Banerjee [ JIRAUSER54513 ]
            Status Stalled [ 10000 ] In Review [ 10002 ]

            I think that the point is that BACKUP STAGE BLOCK_COMMIT allows active transactions to modify InnoDB tables. Copying log records for incomplete transactions would serve no useful purpose. In fact, it would only create extra work when starting up a server on a restored backup, because then the incomplete transactions would have executed more changes that would need to be rolled back.

            marko Marko Mäkelä added a comment - I think that the point is that BACKUP STAGE BLOCK_COMMIT allows active transactions to modify InnoDB tables. Copying log records for incomplete transactions would serve no useful purpose. In fact, it would only create extra work when starting up a server on a restored backup, because then the incomplete transactions would have executed more changes that would need to be rolled back.

            The server could also be busy purging the history of committed InnoDB transactions. Backup should not really care whether such modifications are included. Copying the data files would already have finished at that point, and therefore copying less log will make both backup and applying the log finish faster. The drawback might be that starting a server on the restored backup would potentially have to spend more time purging some history.

            marko Marko Mäkelä added a comment - The server could also be busy purging the history of committed InnoDB transactions. Backup should not really care whether such modifications are included. Copying the data files would already have finished at that point, and therefore copying less log will make both backup and applying the log finish faster. The drawback might be that starting a server on the restored backup would potentially have to spend more time purging some history.
            marko Marko Mäkelä made changes -
            marko Marko Mäkelä made changes -

            Thanks for the patch. Looks good to me.

            debarun Debarun Banerjee added a comment - Thanks for the patch. Looks good to me.
            debarun Debarun Banerjee made changes -
            Assignee Debarun Banerjee [ JIRAUSER54513 ] Marko Mäkelä [ marko ]
            Status In Review [ 10002 ] Stalled [ 10000 ]
            marko Marko Mäkelä made changes -
            Status Stalled [ 10000 ] In Testing [ 10301 ]
            marko Marko Mäkelä made changes -
            Assignee Marko Mäkelä [ marko ] Matthias Leich [ mleich ]
            mleich Matthias Leich made changes -
            Assignee Matthias Leich [ mleich ] Marko Mäkelä [ marko ]
            Status In Testing [ 10301 ] Stalled [ 10000 ]
            marko Marko Mäkelä made changes -
            issue.field.resolutiondate 2024-09-09 14:38:39.0 2024-09-09 14:38:39.082
            marko Marko Mäkelä made changes -
            Fix Version/s 10.11.10 [ 29904 ]
            Fix Version/s 11.2.6 [ 29906 ]
            Fix Version/s 11.4.4 [ 29907 ]
            Fix Version/s 11.6.2 [ 29908 ]
            Fix Version/s 10.11 [ 27614 ]
            Resolution Fixed [ 1 ]
            Status Stalled [ 10000 ] Closed [ 6 ]
            mariadb-jira-automation Jira Automation (IT) made changes -
            Zendesk active tickets 204878
            ralf.gebhardt Ralf Gebhardt made changes -

            People

              marko Marko Mäkelä
              marko Marko Mäkelä
              Votes:
              2 Vote for this issue
              Watchers:
              11 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.