| Bug #52966 | Backup kernel: wrong logic for handling backup drivers. | ||
|---|---|---|---|
| Submitted: | 20 Apr 2010 7:24 | Modified: | 22 Apr 2010 10:24 |
| Reporter: | Rafal Somla | Email Updates: | |
| Status: | Verified | Impact on me: | |
| Category: | MySQL Server: Backup | Severity: | S3 (Non-critical) |
| Version: | mysql-next-mr-backup | OS: | Any |
| Assigned to: | CPU Architecture: | Any | |
[22 Apr 2010 9:50]
Rafal Somla
Here is yet another error in the backup driver handling logic which I report
under this bug.
A backup driver can send DONE reply when sending last chunk of snapshot
data. Currently, the chunk received from the driver is ignored if the driver
sends DONE reply at the same time. This is wrong: backup kernel should save
the chunk in the image and then mark that the driver has finished its job.
Again, this error does not affect the current system since existing backup
drivers never send data and DONE reply together. But it can show up when new
backup drivers are developed.
A fix for this error:
=== modified file 'sql/backup/data_backup.cc'
--- sql/backup/data_backup.cc 2010-04-19 14:08:16 +0000
+++ sql/backup/data_backup.cc 2010-04-22 09:47:59 +0000
@@ -1512,7 +1512,7 @@ int Backup_pump::pump(size_t *howmuch)
state= backup_state::READY;
case OK:
-
+ case DONE:
if (m_buf.last)
{
mark_stream_closed(m_buf.table_num);
@@ -1530,6 +1530,8 @@ int Backup_pump::pump(size_t *howmuch)
else
m_bw.drop_buf(m_buf); // Never errors.
+ if (res == DONE)
+ state= backup_state::DONE;
break;
case PROCESSING:
@@ -1543,8 +1545,6 @@ int Backup_pump::pump(size_t *howmuch)
state= backup_state::ERROR;
return ERROR;
- case DONE:
- state= backup_state::DONE;
case BUSY:
m_bw.drop_buf(m_buf); // Never errors.
@@ -1554,8 +1554,7 @@ int Backup_pump::pump(size_t *howmuch)
} // if (mode == READING)
if (mode == WRITING
- && state != backup_state::ERROR
- && state != backup_state::DONE)
+ && state != backup_state::ERROR)
{
backup::Block_writer::result_t res= m_bw.write_buf(m_buf);
[22 Apr 2010 9:52]
Bugs System
A patch for this bug has been committed. After review, it may be pushed to the relevant source trees for release in the next version. You can access the patch from: http://lists.mysql.com/commits/106346 2948 Rafal Somla 2010-04-22 Fix of the wrong logic in backup kernel for handling DONE reply from a backup driver (see BUG#52966).
[22 Apr 2010 10:24]
Rafal Somla
Changing state - wrong patch picked-up by the bug database.
Description: A READY reply from drv->get_data() method indicates end of initial or prepare phase of the backup synchronization protocol. This is not correctly captured in the Backup_pump::pump() method (data_backup.cc:1540). The logic wrongly intercepts READY reply if the pump is in WAITING state, but it should rather do it when the pump is in PREPARING state. This error in the logic does not affect the current behaviour, because current drivers do not rely on a correct handling of READY reply. But in principle, it is possible that someone writes a backup driver according to the specifications and the system will not work correctly with that driver (this has happened when I worked on a prototype backup driver). How to repeat: Code inspection: Backup_pump::pump() method, after a call to m_drv->get_data() (data_backup.cc:1536). Suggested fix: === modified file 'sql/backup/data_backup.cc' --- sql/backup/data_backup.cc 2009-12-21 07:38:02 +0000 +++ sql/backup/data_backup.cc 2010-04-16 12:23:42 +0000 @@ -1508,7 +1508,7 @@ int Backup_pump::pump(size_t *howmuch) if( state == backup_state::INIT ) state= backup_state::WAITING; - else if( state == backup_state::WAITING ) + else if( state == backup_state::PREPARING ) state= backup_state::READY; case OK: