Menu

The Story of One Contribution

The MySQL Server 5.7.5 Development Milestone Release includes support for acquiring multiple user-level locks within the same connection. The implementation of this feature is based on a contributed patch by Konstantin Osipov. This post tells the story about what happened with this patch on its way into the MySQL Server codebase.

Requests to extend the semantics of user-level locks in MySQL to support acquisition of multiple locks in the same connection have been around at least since 2003 (see bug#1118).

But for a long time we lacked crucial infrastructure for implementing this change. In MySQL Server 5.5 we at last introduced the metadata locking (MDL) subsystem — an engine-independent lock manager with support for deadlock detection. This finally made the implementation of the feature request feasible. Unfortunately, tasks with higher priority took precedence and support for multiple user-level locks didn’t get attention in either the 5.5 or in the 5.6 release.

Naturally, we were really happy when our former colleague Konstantin Osipov came up with a patch implementing this feature and was kind enough to contribute his patch to MySQL Server.

So what happened after the necessary steps (as described in this post by Erlend Dahl)—like signing the OCA, reporting a bug (bug #67806) and adding the patch to it—were all completed by Konstantin?

First of all, even though this was a contribution we still needed to provide a proper design document for it. There are several reasons why:

Having a design document allows us to perform a proper design review without diving too much into implementation details and see if there are issues with the design or if there is some nice-to-have functionality which might be missing.

The design document serves as input for the QA Team for building test plans.

The design document also serves as input for the Documentation Team when time comes to document the changes implemented.

After that the design was reviewed, review comments discussed and adjustments to the document and the design were made. A few examples of issues which we identified while working on the design document for this particular contribution:

It is a bad idea to use ER_LOCK_DEADLOCK for deadlocks caused by user-level locks as this error code implies transaction rollback, which doesn’t happen in the case of user-level locks.

It would be nice to have a function which releases all locks held by a connection. So we decided to add RELEASE_ALL_LOCKS().

Existing code always assumed that lock names are in UTF-8 and compared them in case-insensitive fashion. We decided that we need to convert names to UTF-8 if they are provided in different charset and preserve case-insensitive comparison.

Then the patch was adjusted according to the updated design (some contributions are even totally rewritten as a result of this process) and the code was polished. For example, we decided to use a somewhat different API for finding the lock owner in the metadata locking subsystem instead of the one provided in the patch contributed. Sometimes we might find bugs in the old code during this step. In this case we found bug #73123 “Possible access to freed memory in IS_FREE_LOCK() and IS_USED_LOCK()”.

Around the same time the QA team reviewed the design, provided their comments, and prepared the test plan. Occasionally design and code needs to be changed as result of these discussions. This didn’t happen in this case though. Test cases were extended accordingly.

Code review happened. Issues discovered during it were fixed. Finally, the patch was approved by the reviewer and passed into the hands of the QA team.

The QA team ran the tests according to their test plan (including performance benchmarks and RQG runs) and checked code coverage. Often issues are discovered which need to be fixed at this stage. In this particular case no issues were found. We also checked that the code builds and passes tests on all platforms we support.

Once the QA team said that patch was good enough to go into trunk, it was then pushed into the main MySQL development branch (MySQL 5.7).

2 thoughts on “The Story of One Contribution”

Thanks for merging the patch and for writing this. It’s great to see
non-trivial externally-written code merged to the server.

Were the changes to the contributed patch discussed with Konstantin or
was this purely internal with not even the original code submitter
having an idea what will emerge in the next public server release?

In both cases the process could use some more transparency that I
don’t think would hurt your internal processes: why not discuss the
changes to the design document on the internals mailing list?