Code review comment for lp:~akopytov/percona-server/priority-scheduling-for-threadpool-5.5

Hi Laurynas,

On Wed, 03 Apr 2013 05:23:21 -0000, Laurynas Biveinis wrote:
> Review: Needs Information
>
> - Double comma at the diff line 330.
>

Fixed.

> - If I understand correctly, whenever the connection exhausts its tickets, and is put back into the regular queue, and is woken up, and has to be enqueued again, it goes to the high priority queue with the original number of tickets again. Do you think that this cyclic behavior needs to be elaborated upon a bit more than it is? The sentence "Otherwise the connection is put into the common queue with the initial tickets value specified with this option." sort of does it, but IMHO it's easy to miss, so I'd add a bit more to it, for example "Otherwise the connection is put into the common queue. If, after being taken for processing, it needs to be queued again, it goes to the high priority queue with the initial tickets value specified with this option."
>

That's not technically correct. The correct version would be:

"
Otherwise the connection is put into the common queue. If, after being
taken for processing, it needs to be queued again, it goes to the high
priority queue *if it has an open transaction* with the initial tickets
value specified with this option *decremented by one*.
"

I tried to think of some text that would be both technically correct and
easy to read and came back to my original text :)

> - Would it be a good idea to a very minimal test (i.e. the server doesn't crash with the value set, not for the desired scheduling behavior) to pool_of_threads.test for the tickets > 0 case?
>

Added.

http://jenkins.percona.com/view/PS%205.5/job/percona-server-5.5-param/716/