The Ultimate Guide to High Availability with MariaDB

Contributing Code

You are viewing an old version of this article. View the current version here.

Contents

  1. Finding Development Projects to Work on
  2. What to Expect From a MariaDB Server Developer
    1. The Basics
    2. What to Have in a Commit Comment
    3. Testing
    4. Before Pushing Code to a Stable Branch
    5. Working on a New Project
    6. Working on a Bug Fix
    7. Making Things Easier for Reviewers
    8. When Reviewing Code
  3. See Also

This page contains general guidelines and procedures for contributing code. If you have any questions we invite you to ask on the maria-developers mailing list, on our Zulip instance at https://mariadb.zulipchat.com, or on the #maria IRC channel on https://libera.chat/. Other email lists and places to find MariaDB can be found here.

General information about contributing to MariaDB (for developers and non-developers) is found on the Contributing to the MariaDB Project page.

Finding Development Projects to Work on

There are many open development projects for MariaDB which you can contribute to (in addition to any ideas you may have yourself).

  • We are using JIRA to manage the MariaDB project. Go to https://jira.mariadb.org and click on "Projects" to get to the MariaDB project. Browse around the unresolved and unassigned issues to see if there is something that interests you. Some issues have sponsors and you can be paid for doing them!
  • Join maria-developers and write to the maria-developers \at\ lists.launchpad.net list and ask for suggestions of tasks you could do. Please include your programming experience and your knowledge of the MariaDB source and how much you know about using MySQL/MariaDB with the email so that we know which tasks we can suggest to you.
  • If this is your first project, check out the Suggested Development page. It lists projects that will make a good start.

If you have your own ideas, please submit them to JIRA so other MariaDB developers can comment on them and suggest how to implement them. You can of course also use the maria-developers list for this.

What to Expect From a MariaDB Server Developer

This section is mainly directed to developers with commit rights to the MariaDB git repository. However, we hope it’s also useful for anyone wanting to contribute code to MariaDB to know what a reviewer will expect from them.

This is not about coding style or if one should prefer C instead of C++. That would be a separate topic that should be created sooner or later.

The Basics

When coding, try to create code that 'never has to be changed again'. Try to make the code as performant as possible. In general it is acceptable to spend 50% more time to make the code 15% faster than what you originally intended. Take that into account when you plan your time estimates! That said, don't try to add classes or functionality that is not yet used.

The code should be easy to read and follow the coding standards of the project. Patches that are smaller and simpler are often better than complex solutions. Don't make the server depend on new external libraries without first checking with Sergei or Monty!

Add code comments for anything that is not obvious. When possible, use assertions within the code to document expectations of arguments etc. In general, if the code requires complex comments, think if there is a better way to structure the logic. Simpler is often better and with fewer bugs.

What to Have in a Commit Comment

  • Jira issue number and summary ex: MDEV-23839 innodb_fast_shutdown=0 hang on change buffer merge
  • An empty line
  • A short description of the problem
  • A description of the solution
  • Any extra information needed to understand the patch
  • The commit message should be self contained and the reviewer shouldn't preferably have to look at the Jira at all to understand the commit. This doesn’t mean that the commit message should include all background and different design options considered, as the Jira should contain.
  • Name of all reviewers and authors should be clear from the commit message. The preferred way would be (one line per person)
  • The default is that all code should be reviewed. Only in really extraordinary cases, like merge (where the original code was already reviewed) then it can be self-reviewed, which should clear from the commit. In this case the code should of course be tested extra carefully both locally and in buildbot before pushing.

Testing

  • All code should have a test case that shows that the new code works or, in case of a bug fix, that the problem is fixed! It should fail with an unpatched server and work with the new version. In the extreme case that a test case is practically impossible to do, there needs to be documentation (in the commit message, optionally also in Jira) how the code was tested.
  • The test case should have a reference to the Jira issue, if such one exists.
  • Patches related to performance should be tested either by the developer (for simple commits) or by performance testers. The result should be put in Jira with a summary in the commit.
  • Complex patches and should be tested by QA in a bb- branch before pushing. The Jira entry should include information that this has been done and what kind of test has been run.
  • For anything not trivial, one should run either Valgrind or ASAN/MSAN on the new code. (Buildbot will do this for you if you can’t get valgrind or ASAN to work). At least the test case added should be tested by either. If the developer cannot do that for some reason, he should check the buildbot builders that do this and ensure that at least his test case doesn’t give any warnings about using not initialized memory or other failures.
  • For complex code the developer should preferably use gcov or some similar tool to ensure that at least not all not-error branches are executed. “mtr --gcov” or “dgcov.pl” can help you with this.

Before Pushing Code to a Stable Branch

  • Ensure that you have compiled everything for your new code, in a debug server (configured with cmake -DCMAKE_BUILD_TYPE=Debug ) including embedded and all plugins that may be affected by your code change..
  • Run the mysql-test-run (mtr) test suite locally with your debug server.
  • For anything complex the full test suite should be run.
  • For something absolutely trivial, at least the main suite must be run.
  • Always push first to a bb- branch to test the code. When the bb- branch is green in buildbot you can push to the main branch. Take care of checking that Windows builds compiles (take extra care of checking this as this often fails) and that valgrind and msan builds doesn’t show any problems with your new test cases.
  • If you have to do a rebase before pushing, you have to start from the beginning again.
  • When porting code from third parties (such as MySQL), make sure to attribute copyright to the right owner, in the header of each modified file.
  • For example: Copyright © 2000, 2018, Oracle and/or its affiliates. Copyright © 2009, 2020, MariaDB
  • The only exception is that if the changes are trivial and the rebase was trivial and the local mysql-test-run worked, then you can push directly to the main branch. Only do this if you are 99% sure there are no issues! * Please don't make us regret that we have made this one exception! When we have protected git branches, then the above rule will be enforced automatically as the protection will take care of this.

Working on a New Project

  • First create a Jira entry that explains the problems and the different solutions that can be used to solve the problem. If there is a new syntax include examples of queries and results.
  • After getting an agreement of the to-be-used solution, update the Jira entry with the detailed architecture of the suggested solution.
  • When the architecture is reviewed, the assigned developer can start coding.
  • When the code is ready, the Jira entry should be updated with the reviewer.
  • The reviewer checks the code and either approves it to be pushed or gives comments to the developers that should be fixed. In the later case the developer updates the code and gives it back to the reviewer. This continues until the code is approved.
  • If the design changes during the project, the design in Jira needs to be updated.

Working on a Bug Fix

  • Ensure that the Jira issue is up to date.
  • For complex bugs that require redesign, follow the process in "Working on a new project"
  • For simpler bugs, one can skip the listing of different solutions and architecture. However one should still document the reason for the bug and how it's fixed or to-be-fixed, in a JIRA comment.

Making Things Easier for Reviewers

  • Ensure that code compiles, all MTR test works before asking for a review
  • Try to split a bigger project into smaller, self-contained change sets.
  • Automatic things, like renames of classes, variables, functions etc is better to be done in a separate commit.

When Reviewing Code

Remember that the stability and security of any project hangs a lot on the reviewers. If there is something wrong with an accepted patch, it's usually the reviewer who is to be blamed for it, as the reviewer was the one who allowed it to go in!

  • Ensure that the code is licensed under New BSD or another approved license for MariaDB (basically any open source license not conflicting with GPL) or that the contributor has signed the MCA.
  • GPL is only allowed for code from MySQL (as MariaDB is already depending on MySQL code).
  • Ensure that commits are not too large. If the code is very large, give suggestions how to split it into smaller pieces. Merge commits, when rebasing is possible, are not allowed, to keep history linear.
  • Check that the commits message describes the commit properly. For code that improves performance, ensure that Jira and the commit message contains information about the improvements.
  • Check that there are no unexplained changes in old tests.
  • Check the quality of the code (no obvious bugs, right algorithms used)
  • Check if any code can be simplified or optimized. Using already existing functions, are loops optimal, are mutexes used correctly etc.
  • Check that there is an appropriate test case for the code. See ‘testing’ for what is required!
  • Ensuring the code follows the coding standard for MariaDB. This document should be created shortly, but in the meantime ask an old MySQL/MariaDB developer if you are unsure.
  • Ensuring that the code follows the architecture agreed for in Jira (if it's in Jira).
  • Code should be easy to understand (good code comments, good function and variable names etc).
  • Ensure you understand every single line of code that is reviewed. If not, ask the developer to add more comments to get things clear or ask help from another reviewer.
  • No performance degradations for all common cases.
  • Any code that touches any sensitive area (files, communication, login, encryption or security) needs to have another reviewer that is an expert in this area.

See Also

Comments

Comments loading...
Content reproduced on this site is the property of its respective owners, and this content is not reviewed in advance by MariaDB. The views, information and opinions expressed by this content do not necessarily represent those of MariaDB or any other party.