Merge lp:~gaul/percona-server/pwrite_partial_write into lp:percona-server/5.6

Proposed by Andrew Gaul
Status: Rejected
Rejected by: Laurynas Biveinis
Proposed branch: lp:~gaul/percona-server/pwrite_partial_write
Merge into: lp:percona-server/5.6
Diff against target: 20 lines (+4/-3)
1 file modified
Percona-Server/storage/innobase/os/os0file.cc (+4/-3)
To merge this branch: bzr merge lp:~gaul/percona-server/pwrite_partial_write
Reviewer Review Type Date Requested Status
Laurynas Biveinis (community) Needs Resubmitting
Review via email: mp+199611@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Laurynas Biveinis (laurynas-biveinis) wrote :

Andrew -

Thanks for the patch. It is correct in general, I'm testing it with two additional changes:
- replace non-portable void* pointer arithmetics with an expression through casting to char*;
- the same bug is present in os_file_pread().

The same bug is present in os_file_pread()/os_file_pwrite() for systems that do not support pwrite()/pread(), e.g. Windows. We don't support any such platform, thus I am ignoring it for the now.

I'll attach the updated patch merge proposal to the bug report.

review: Needs Resubmitting

Unmerged revisions

519. By Andrew Gaul <email address hidden>

Handle partial writes in os0file.cc:os_file_pwrite

This could cause ibd files to contain bogus trailing zeros and non-page
size file sizes, which XtraBackup asserts on.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'Percona-Server/storage/innobase/os/os0file.cc'
--- Percona-Server/storage/innobase/os/os0file.cc 2013-12-05 17:23:10 +0000
+++ Percona-Server/storage/innobase/os/os0file.cc 2013-12-19 05:43:39 +0000
@@ -2624,12 +2624,13 @@
2624 MONITOR_ATOMIC_INC(MONITOR_OS_PENDING_WRITES);2624 MONITOR_ATOMIC_INC(MONITOR_OS_PENDING_WRITES);
2625#endif /* !HAVE_ATOMIC_BUILTINS || UNIV_WORD < 8 */2625#endif /* !HAVE_ATOMIC_BUILTINS || UNIV_WORD < 8 */
26262626
2627 /* Handle signal interruptions correctly */2627 /* Handle partial writes and signal interruptions correctly */
2628 for (ret = 0; ret < (ssize_t) n; ) {2628 for (ret = 0; ret < (ssize_t) n; ) {
2629 n_written = pwrite(file, buf, (ssize_t)n, offs);2629 n_written = pwrite(file, buf, ((ssize_t)n) - ret, offs);
2630 if (n_written > 0) {2630 if (n_written >= 0) {
2631 ret += n_written;2631 ret += n_written;
2632 offs += n_written;2632 offs += n_written;
2633 buf += n_written;
2633 } else if (n_written == -1 && errno == EINTR) {2634 } else if (n_written == -1 && errno == EINTR) {
2634 continue;2635 continue;
2635 } else {2636 } else {

Subscribers

People subscribed via source and target branches