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

Revision history for this message
Laurynas Biveinis (laurynas-biveinis) wrote :

- Double comma at the diff line 330.

- 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."

- 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?

review: Needs Information

« Back to merge proposal