Scylla Documentation Logo Documentation
  • Server
    • Scylla Open Source
    • Scylla Enterprise
    • Scylla Alternator
  • Cloud
    • Scylla Cloud
    • Scylla Cloud Docs
  • Tools
    • Scylla Manager
    • Scylla Monitoring Stack
    • Scylla Operator
  • Drivers
    • CQL Drivers
    • DynamoDB Drivers
Download
Menu
Scylla Dev Contribute Review Checklist

Review Checklist¶

How to use this document¶

This document describes general guidelines for reviewing patches to Scylla. The guidelines are not absolute; with good reason they can be overridden at the discretion of the maintainers. Nevertheless, patches should try to conform to these guidelines instead of finding reasons for seeking an exception.

Code style¶

Code should conform to the project’s coding style, see Seastar coding style.

Naming¶

Names should avoid abbreviations (industry-standard abbreviations are acceptable) as they make the code opaque to newcomers. Within a restricted scope such as a function or a class, local or private variables can be given abbreviated names.

Micro-performance¶

The following should be avoided:

  • computational loops without preemption checks and yields

  • allocation of large contiguous memory blocks in non-initialization code

  • quadratic (or worse) behavior on user-controlled inputs

Concurrency¶

Code that generates concurrent behavior should include means to control the amount of concurrency, placing an upper bound on the amount of memory in use by concurrent operations.

If a component performs work in the background, it would include a mechanism (stop() or close()) to wait for background work to complete. If background work is unbounded in time, the code should be able to abort in-progress work.

Unit testing¶

Patches should be accompanied by unit test when feasible.

  • patches that fix a bug should be accompanied by a test that fails before the patch is applied and passes after it is applied

  • patches that introduce generic code (such as a container) should test all aspects of that code, not just those that are used by the patchset.

Templates¶

In non performance sensitive code, type erasure should be preferred to templates in order to reduce code and compile time bloat. In performance sensitive code, template bloat can be justified.

Template parameters should be constrained by C++ concepts, both as code documentation, and to enable early error checking by the compiler.

Templates should not be used as an ad-hoc method to reduce code duplication; rather they should be used when a common concept can be captured in template code and reused.

Singletons¶

The singleton pattern should be avoided when it can introduce confusion about initialization order or make unit testing difficult. Instead, dependencies should be passed via constructor parameters (the “dependency injection” pattern).

Invariant checking in scylla¶

Non obvious invariants should be checked. There are three ways of doing it, asserts, throwing exceptions and logging failures.

If an invariant is critical for the stability of the system, use an assert. For example, a broken invariant in the memory allocator can lead to data corruption and there is no way to work around it, so assert.

If an invariant is needed only by some features and the system can shutdown cleanly without it, then a throw is appropriate. If the issue was found while serving a user request the exception is propagated back to the user. It if was a scylla internal operation, it will still bring down the node.

When the operation can just be ignored, logging the failure is the best option. The canonical example is when an element is “known” to be in a container, but while trying to remove it we find out that it was not there.

PREVIOUS
Maintainer’s handbook
NEXT
Service Level Distributed Data
  • 4.6
    • 4.6
    • 4.5
  • Scylla Developer Documentation
  • Alternator: DynamoDB API in Scylla
    • Getting Started With ScyllaDB Alternator
    • Scylla Alternator for DynamoDB users
  • Design Notes
    • IDL compiler
    • CDC
    • The Compaction Controller
    • Scylla CQL extensions
    • Scylla CQL extensions
    • CQL3 Type Mapping
    • Hinted Handoff Design
    • Performance Isolation in Scylla
    • CQL to Lua type mapping
    • Scylla Metrics
    • Migrating from users to roles
    • Paged queries
    • Protocol extensions to the Cassandra Native Protocol
    • Ports and protocols in Scylla
    • Redis API in Scylla
    • Repair based node operations
    • Reverse reads
    • Row Cache
    • Row level repair
    • Secondary indexes in Scylla
    • File format of the Scylla.db sstable component
    • sstables directory structure
    • System keyspace layout
    • System schema keyspace layout
    • WASM support for user-defined functions
  • Guides
    • Guidelines for developing Scylla
    • Scylla RESTful API V2
    • Building Scylla
    • Debugging with GDB
    • Docker Hub Image
    • Logging in Scylla
    • Testing
    • Tracing
  • Contribute
    • Contributing to Scylla
    • Backport
    • Maintainer’s handbook
    • Review Checklist
  • Service Level Distributed Data
  • Create an issue
  • Edit this page

On this page

  • Review Checklist
    • How to use this document
    • Code style
    • Naming
    • Micro-performance
    • Concurrency
    • Unit testing
    • Templates
    • Singletons
    • Invariant checking in scylla
Logo
Docs Contact Us About Us
Mail List Icon Slack Icon
© 2022, ScyllaDB. All rights reserved.
Last updated on 12 May 2022.
Powered by Sphinx 4.3.2 & ScyllaDB Theme 1.2.1