Merge lp:~stewart/percona-server/5.5.27-plus-bugfixes into lp:percona-server/5.5

Proposed by Stewart Smith on 2012-08-10
Status: Work in progress
Proposed branch: lp:~stewart/percona-server/5.5.27-plus-bugfixes
Merge into: lp:percona-server/5.5
Prerequisite: lp:~stewart/percona-server/5.5.27
Diff against target: 55 lines (+15/-6)
1 file modified
Percona-Server/storage/innobase/buf/buf0lru.c (+15/-6)
To merge this branch: bzr merge lp:~stewart/percona-server/5.5.27-plus-bugfixes
Reviewer Review Type Date Requested Status
Alexey Kopytov (community) Needs Information on 2012-08-15
Laurynas Biveinis (community) 2012-08-10 Needs Information on 2012-08-10
Review via email: mp+119085@code.launchpad.net

Description of the change

Include critical bugfixes based on Alexey's feedback to initial 5.5.27 branch. This merge req has that branch as a prerequisite.

http://jenkins.percona.com/job/percona-server-5.5-param/461/

To post a comment you must log in.

Does this apply to 5.1? (If it does, should be fixed there first and merged up...)

Why did you add the "impossible case" if (!block_mutex) ... ?

review: Needs Information
Stewart Smith (stewart) wrote :

Laurynas Biveinis <email address hidden> writes:

> Review: Needs Information
>
> Does this apply to 5.1? (If it does, should be fixed there first and
> merged up...)

Haven't checked to be honest :)

>
> Why did you add the "impossible case" if (!block_mutex) ... ?

It was there in original code that was missed during merge.... it is a
bit odd though.

--
Stewart Smith

> > Does this apply to 5.1? (If it does, should be fixed there first and
> > merged up...)
>
> Haven't checked to be honest :)

OK if the fixes do not apply for 5.1. If they do, then 5.1 has to be fixed and upmerged.

Stewart Smith (stewart) wrote :

> > > Does this apply to 5.1? (If it does, should be fixed there first and
> > > merged up...)
> >
> > Haven't checked to be honest :)
>
> OK if the fixes do not apply for 5.1. If they do, then 5.1 has to be fixed and
> upmerged.

FYI they don't seem to apply to 5.1 from code inspection.

Alexey Kopytov (akopytov) wrote :

I don't the "It may be impossible case" code in the original fix. The upstream code is:

---
  } else {

   block_mutex = buf_page_get_mutex(bpage);
   mutex_enter(block_mutex);

   if (bpage->buf_fix_count > 0) {

    mutex_exit(block_mutex);

    /* We cannot remove this page during
    this scan yet; maybe the system is
    currently reading it in, or flushing
    the modifications to the file */

    all_freed = FALSE;

    goto next_page;
   }
  }

---

Which appears to be rather different than what is now in Percona Server. Why?

review: Needs Information

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'Percona-Server/storage/innobase/buf/buf0lru.c'
2--- Percona-Server/storage/innobase/buf/buf0lru.c 2012-08-10 06:51:31 +0000
3+++ Percona-Server/storage/innobase/buf/buf0lru.c 2012-08-10 06:51:33 +0000
4@@ -472,7 +472,7 @@
5 mutex_t* block_mutex;
6 ibool processed = FALSE;
7
8- ut_ad(buf_pool_mutex_own(buf_pool));
9+ ut_ad(mutex_own(&buf_pool->LRU_list_mutex));
10 ut_ad(buf_flush_list_mutex_own(buf_pool));
11
12 block_mutex = buf_page_get_mutex(bpage);
13@@ -595,11 +595,11 @@
14 ibool all_freed;
15
16 do {
17- buf_pool_mutex_enter(buf_pool);
18+ mutex_enter(&buf_pool->LRU_list_mutex);
19
20 all_freed = buf_flush_or_remove_pages(buf_pool, id);
21
22- buf_pool_mutex_exit(buf_pool);
23+ mutex_exit(&buf_pool->LRU_list_mutex);
24
25 ut_ad(buf_flush_validate(buf_pool));
26
27@@ -659,8 +659,16 @@
28 goto next_page;
29 } else {
30
31- block_mutex = buf_page_get_mutex(bpage);
32- mutex_enter(block_mutex);
33+ block_mutex = buf_page_get_mutex_enter(bpage);
34+
35+ if (!block_mutex) {
36+ /* It may be impossible case...
37+ Something wrong, so will be scan_again */
38+
39+ all_freed = FALSE;
40+ goto next_page;
41+ }
42+
43
44 if (bpage->buf_fix_count > 0) {
45
46@@ -694,7 +702,8 @@
47 ulint page_no;
48 ulint zip_size;
49
50- buf_pool_mutex_exit(buf_pool);
51+ mutex_exit(&buf_pool->LRU_list_mutex);
52+ rw_lock_x_unlock(&buf_pool->page_hash_latch);
53
54 zip_size = buf_page_get_zip_size(bpage);
55 page_no = buf_page_get_page_no(bpage);

Subscribers

People subscribed via source and target branches