| <!DOCTYPE html> |
| <html lang="en"> |
| <head> |
| <meta charset="utf-8"> |
| <meta http-equiv="X-UA-Compatible" content="IE=edge"> |
| <meta name="viewport" content="width=device-width, initial-scale=1"> |
| <!-- The above 3 meta tags *must* come first in the head; any other head content must come *after* these tags --> |
| <title>Apache Flink: Apache Flink Code Style and Quality Guide — Common Rules</title> |
| <link rel="shortcut icon" href="/favicon.ico" type="image/x-icon"> |
| <link rel="icon" href="/favicon.ico" type="image/x-icon"> |
| |
| <!-- Bootstrap --> |
| <link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/bootstrap/3.4.1/css/bootstrap.min.css"> |
| <link rel="stylesheet" href="/css/flink.css"> |
| <link rel="stylesheet" href="/css/syntax.css"> |
| |
| <!-- Blog RSS feed --> |
| <link href="/blog/feed.xml" rel="alternate" type="application/rss+xml" title="Apache Flink Blog: RSS feed" /> |
| |
| <!-- jQuery (necessary for Bootstrap's JavaScript plugins) --> |
| <!-- We need to load Jquery in the header for custom google analytics event tracking--> |
| <script src="/js/jquery.min.js"></script> |
| |
| <!-- HTML5 shim and Respond.js for IE8 support of HTML5 elements and media queries --> |
| <!-- WARNING: Respond.js doesn't work if you view the page via file:// --> |
| <!--[if lt IE 9]> |
| <script src="https://oss.maxcdn.com/html5shiv/3.7.2/html5shiv.min.js"></script> |
| <script src="https://oss.maxcdn.com/respond/1.4.2/respond.min.js"></script> |
| <![endif]--> |
| </head> |
| <body> |
| |
| |
| <!-- Main content. --> |
| <div class="container"> |
| <div class="row"> |
| |
| |
| <div id="sidebar" class="col-sm-3"> |
| |
| |
| <!-- Top navbar. --> |
| <nav class="navbar navbar-default"> |
| <!-- The logo. --> |
| <div class="navbar-header"> |
| <button type="button" class="navbar-toggle collapsed" data-toggle="collapse" data-target="#bs-example-navbar-collapse-1"> |
| <span class="icon-bar"></span> |
| <span class="icon-bar"></span> |
| <span class="icon-bar"></span> |
| </button> |
| <div class="navbar-logo"> |
| <a href="/"> |
| <img alt="Apache Flink" src="/img/flink-header-logo.svg" width="147px" height="73px"> |
| </a> |
| </div> |
| </div><!-- /.navbar-header --> |
| |
| <!-- The navigation links. --> |
| <div class="collapse navbar-collapse" id="bs-example-navbar-collapse-1"> |
| <ul class="nav navbar-nav navbar-main"> |
| |
| <!-- First menu section explains visitors what Flink is --> |
| |
| <!-- What is Stream Processing? --> |
| <!-- |
| <li><a href="/streamprocessing1.html">What is Stream Processing?</a></li> |
| --> |
| |
| <!-- What is Flink? --> |
| <li><a href="/flink-architecture.html">What is Apache Flink?</a></li> |
| |
| |
| |
| <!-- What is Stateful Functions? --> |
| |
| <li><a href="/stateful-functions.html">What is Stateful Functions?</a></li> |
| |
| <!-- Use cases --> |
| <li><a href="/usecases.html">Use Cases</a></li> |
| |
| <!-- Powered by --> |
| <li><a href="/poweredby.html">Powered By</a></li> |
| |
| |
| |
| <!-- Second menu section aims to support Flink users --> |
| |
| <!-- Downloads --> |
| <li><a href="/downloads.html">Downloads</a></li> |
| |
| <!-- Getting Started --> |
| <li class="dropdown"> |
| <a class="dropdown-toggle" data-toggle="dropdown" href="#">Getting Started<span class="caret"></span></a> |
| <ul class="dropdown-menu"> |
| <li><a href="https://ci.apache.org/projects/flink/flink-docs-release-1.11/getting-started/index.html" target="_blank">With Flink <small><span class="glyphicon glyphicon-new-window"></span></small></a></li> |
| <li><a href="https://ci.apache.org/projects/flink/flink-statefun-docs-release-2.1/getting-started/project-setup.html" target="_blank">With Flink Stateful Functions <small><span class="glyphicon glyphicon-new-window"></span></small></a></li> |
| <li><a href="/training.html">Training Course</a></li> |
| </ul> |
| </li> |
| |
| <!-- Documentation --> |
| <li class="dropdown"> |
| <a class="dropdown-toggle" data-toggle="dropdown" href="#">Documentation<span class="caret"></span></a> |
| <ul class="dropdown-menu"> |
| <li><a href="https://ci.apache.org/projects/flink/flink-docs-release-1.11" target="_blank">Flink 1.11 (Latest stable release) <small><span class="glyphicon glyphicon-new-window"></span></small></a></li> |
| <li><a href="https://ci.apache.org/projects/flink/flink-docs-master" target="_blank">Flink Master (Latest Snapshot) <small><span class="glyphicon glyphicon-new-window"></span></small></a></li> |
| <li><a href="https://ci.apache.org/projects/flink/flink-statefun-docs-release-2.1" target="_blank">Flink Stateful Functions 2.1 (Latest stable release) <small><span class="glyphicon glyphicon-new-window"></span></small></a></li> |
| <li><a href="https://ci.apache.org/projects/flink/flink-statefun-docs-master" target="_blank">Flink Stateful Functions Master (Latest Snapshot) <small><span class="glyphicon glyphicon-new-window"></span></small></a></li> |
| </ul> |
| </li> |
| |
| <!-- getting help --> |
| <li><a href="/gettinghelp.html">Getting Help</a></li> |
| |
| <!-- Blog --> |
| <li><a href="/blog/"><b>Flink Blog</b></a></li> |
| |
| |
| <!-- Flink-packages --> |
| <li> |
| <a href="https://flink-packages.org" target="_blank">flink-packages.org <small><span class="glyphicon glyphicon-new-window"></span></small></a> |
| </li> |
| |
| |
| <!-- Third menu section aim to support community and contributors --> |
| |
| <!-- Community --> |
| <li><a href="/community.html">Community & Project Info</a></li> |
| |
| <!-- Roadmap --> |
| <li><a href="/roadmap.html">Roadmap</a></li> |
| |
| <!-- Contribute --> |
| <li><a href="/contributing/how-to-contribute.html">How to Contribute</a></li> |
| |
| <ul class="nav navbar-nav navbar-subnav"> |
| <li > |
| <a href="/contributing/contribute-code.html">Contribute Code</a> |
| </li> |
| <li > |
| <a href="/contributing/reviewing-prs.html">Review Pull Requests</a> |
| </li> |
| <li > |
| <a href="/contributing/code-style-and-quality-preamble.html">Code Style and Quality Guide</a> |
| </li> |
| <li > |
| <a href="/contributing/contribute-documentation.html">Contribute Documentation</a> |
| </li> |
| <li > |
| <a href="/contributing/docs-style.html">Documentation Style Guide</a> |
| </li> |
| <li > |
| <a href="/contributing/improve-website.html">Contribute to the Website</a> |
| </li> |
| </ul> |
| |
| |
| <!-- GitHub --> |
| <li> |
| <a href="https://github.com/apache/flink" target="_blank">Flink on GitHub <small><span class="glyphicon glyphicon-new-window"></span></small></a> |
| </li> |
| |
| |
| |
| <!-- Language Switcher --> |
| <li> |
| |
| |
| <a href="/zh/contributing/code-style-and-quality-common.html">中文版</a> |
| |
| |
| </li> |
| |
| </ul> |
| |
| <ul class="nav navbar-nav navbar-bottom"> |
| <hr /> |
| |
| <!-- Twitter --> |
| <li><a href="https://twitter.com/apacheflink" target="_blank">@ApacheFlink <small><span class="glyphicon glyphicon-new-window"></span></small></a></li> |
| |
| <!-- Visualizer --> |
| <li class=" hidden-md hidden-sm"><a href="/visualizer/" target="_blank">Plan Visualizer <small><span class="glyphicon glyphicon-new-window"></span></small></a></li> |
| |
| <hr /> |
| |
| <li><a href="https://apache.org" target="_blank">Apache Software Foundation <small><span class="glyphicon glyphicon-new-window"></span></small></a></li> |
| |
| <li> |
| <style> |
| .smalllinks:link { |
| display: inline-block !important; background: none; padding-top: 0px; padding-bottom: 0px; padding-right: 0px; min-width: 75px; |
| } |
| </style> |
| |
| <a class="smalllinks" href="https://www.apache.org/licenses/" target="_blank">License</a> <small><span class="glyphicon glyphicon-new-window"></span></small> |
| |
| <a class="smalllinks" href="https://www.apache.org/security/" target="_blank">Security</a> <small><span class="glyphicon glyphicon-new-window"></span></small> |
| |
| <a class="smalllinks" href="https://www.apache.org/foundation/sponsorship.html" target="_blank">Donate</a> <small><span class="glyphicon glyphicon-new-window"></span></small> |
| |
| <a class="smalllinks" href="https://www.apache.org/foundation/thanks.html" target="_blank">Thanks</a> <small><span class="glyphicon glyphicon-new-window"></span></small> |
| </li> |
| |
| </ul> |
| </div><!-- /.navbar-collapse --> |
| </nav> |
| |
| </div> |
| <div class="col-sm-9"> |
| <div class="row-fluid"> |
| <div class="col-sm-12"> |
| <h1>Apache Flink Code Style and Quality Guide — Common Rules</h1> |
| |
| |
| <ul class="list-group" style="padding-top: 30px; font-weight: bold;"> |
| <li class="list-group-item"> |
| <a href="/contributing/code-style-and-quality-preamble.html"> |
| Preamble |
| </a> |
| </li> |
| <li class="list-group-item"> |
| <a href="/contributing/code-style-and-quality-pull-requests.html"> |
| Pull Requests & Changes |
| </a> |
| </li> |
| <li class="list-group-item"> |
| <a href="/contributing/code-style-and-quality-common.html"> |
| Common Coding Guide |
| </a> |
| </li> |
| <li class="list-group-item"> |
| <a href="/contributing/code-style-and-quality-java.html"> |
| Java Language Guide |
| </a> |
| </li> |
| <li class="list-group-item"> |
| <a href="/contributing/code-style-and-quality-scala.html"> |
| Scala Language Guide |
| </a> |
| </li> |
| <li class="list-group-item"> |
| <a href="/contributing/code-style-and-quality-components.html"> |
| Component Guides |
| </a> |
| </li> |
| <li class="list-group-item"> |
| <a href="/contributing/code-style-and-quality-formatting.html"> |
| Formatting Guide |
| </a> |
| </li> |
| </ul> |
| |
| <hr /> |
| |
| <div class="page-toc"> |
| <ul id="markdown-toc"> |
| <li><a href="#copyright" id="markdown-toc-copyright">1. Copyright</a></li> |
| <li><a href="#tools" id="markdown-toc-tools">2. Tools</a> <ul> |
| <li><a href="#warnings" id="markdown-toc-warnings">Warnings</a></li> |
| </ul> |
| </li> |
| <li><a href="#comments-and-code-readability" id="markdown-toc-comments-and-code-readability">3. Comments And Code Readability</a> <ul> |
| <li><a href="#comments" id="markdown-toc-comments">Comments</a></li> |
| <li><a href="#branches-and-nesting" id="markdown-toc-branches-and-nesting">Branches and Nesting</a></li> |
| </ul> |
| </li> |
| <li><a href="#design-and-structure" id="markdown-toc-design-and-structure">4. Design and Structure</a> <ul> |
| <li><a href="#immutability-and-eager-initialization" id="markdown-toc-immutability-and-eager-initialization">Immutability and Eager Initialization</a></li> |
| <li><a href="#nullability-of-the-mutable-parts" id="markdown-toc-nullability-of-the-mutable-parts">Nullability of the Mutable Parts</a></li> |
| <li><a href="#avoid-code-duplication" id="markdown-toc-avoid-code-duplication">Avoid Code Duplication</a></li> |
| <li><a href="#design-for-testability" id="markdown-toc-design-for-testability">Design for Testability</a></li> |
| <li><a href="#performance-awareness" id="markdown-toc-performance-awareness">Performance Awareness</a></li> |
| </ul> |
| </li> |
| <li><a href="#concurrency-and-threading" id="markdown-toc-concurrency-and-threading">5. Concurrency and Threading</a></li> |
| <li><a href="#dependencies-and-modules" id="markdown-toc-dependencies-and-modules">6. Dependencies and Modules</a></li> |
| </ul> |
| |
| </div> |
| |
| <h2 id="copyright">1. Copyright</h2> |
| |
| <p>Each file must include the Apache license information as a header.</p> |
| |
| <div class="highlight"><pre><code>/* |
| * Licensed to the Apache Software Foundation (ASF) under one |
| * or more contributor license agreements. See the NOTICE file |
| * distributed with this work for additional information |
| * regarding copyright ownership. The ASF licenses this file |
| * to you under the Apache License, Version 2.0 (the |
| * "License"); you may not use this file except in compliance |
| * with the License. You may obtain a copy of the License at |
| * |
| * http://www.apache.org/licenses/LICENSE-2.0 |
| * |
| * Unless required by applicable law or agreed to in writing, software |
| * distributed under the License is distributed on an "AS IS" BASIS, |
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. |
| * See the License for the specific language governing permissions and |
| * limitations under the License. |
| */ |
| </code></pre></div> |
| |
| <h2 id="tools">2. Tools</h2> |
| |
| <p>We recommend to follow the <a href="https://ci.apache.org/projects/flink/flink-docs-master/flinkDev/ide_setup.html#checkstyle-for-java">IDE Setup Guide</a> to get IDE tooling configured.</p> |
| |
| <!--- |
| ### Use inspections in IntelliJ |
| |
| * Import the inspections settings into the IDE (see IDE setup guide) |
| * TODO: Need to agree on a profile and export it (like checkstyle) |
| * Write the code such that inspection warnings are addressed |
| * There are few exceptions where an inspection warning is not meaningful. In that case, suppress the inspection warning. |
| --> |
| |
| <h3 id="warnings">Warnings</h3> |
| |
| <ul> |
| <li>We strive for zero warnings</li> |
| <li>Even though there are many warnings in existing code, new changes should not add any additional compiler warnings</li> |
| <li>If it is not possible to address the warning in a sane way (in some cases when working with generics) add an annotation to suppress the warning</li> |
| <li>When deprecating methods, check that this does not introduce additional warnings</li> |
| </ul> |
| |
| <h2 id="comments-and-code-readability">3. Comments And Code Readability</h2> |
| |
| <h3 id="comments">Comments</h3> |
| |
| <p><strong>Golden rule: Comment as much as necessary to support code understanding, but don’t add redundant information.</strong></p> |
| |
| <p>Think about</p> |
| |
| <ul> |
| <li><span style="text-decoration:underline;">What</span> is the code doing?</li> |
| <li><span style="text-decoration:underline;">How</span> does the code do this?</li> |
| <li><span style="text-decoration:underline;">Why</span> is the code like that?</li> |
| </ul> |
| |
| <p>The code alone should explain as much as possible the “<span style="text-decoration:underline;">what</span>” and the “<span style="text-decoration:underline;">how</span>”</p> |
| |
| <ul> |
| <li>Use JavaDocs to describe the roles of classes and the contracts of methods, in cases where the contract is not obvious or intuitive from the method name (the “what”).</li> |
| <li>The flow of the code should give a good description of the “how”. |
| Think of variable and method names as part of the code documenting itself.</li> |
| <li>It often makes reading the code easier if larger blocks that form a unit are moved into a private method with a descriptive name of what that block is doing</li> |
| </ul> |
| |
| <p>In-code comments help explain the <span style="text-decoration:underline;">“why”</span></p> |
| |
| <ul> |
| <li>For example <code>// this specific code layout helps the JIT to better do this or that</code></li> |
| <li>Or <code>// nulling out this field here means future write attempts are fail-fast</code></li> |
| <li>Or <code>// for arguments with which this method is actually called, this seemingly naive approach works actually better than any optimized/smart version</code></li> |
| </ul> |
| |
| <p>In-code comments should not state redundant information about the “what” and “how” that is already obvious in the code itself.</p> |
| |
| <p>JavaDocs should not state meaningless information (just to satisfy the Checkstyle checker).</p> |
| |
| <p><strong>Don’t:</strong></p> |
| |
| <div class="highlight"><pre><code>/** |
| * The symbol expression. |
| */ |
| public class CommonSymbolExpression {} |
| </code></pre></div> |
| <p><strong>Do:</strong></p> |
| |
| <div class="highlight"><pre><code>/** |
| * An expression that wraps a single specific symbol. |
| * A symbol could be a unit, an alias, a variable, etc. |
| */ |
| public class CommonSymbolExpression {} |
| </code></pre></div> |
| |
| <h3 id="branches-and-nesting">Branches and Nesting</h3> |
| |
| <p>Avoid deep nesting of scopes, by flipping the if condition and exiting early.</p> |
| |
| <p><strong>Don’t:</strong></p> |
| |
| <div class="highlight"><pre><code>if (a) { |
| if (b) { |
| if (c) { |
| the main path |
| } |
| } |
| } |
| </code></pre></div> |
| |
| <p><strong>Do</strong></p> |
| |
| <div class="highlight"><pre><code>if (!a) { |
| return .. |
| } |
| |
| if (!b) { |
| return ... |
| } |
| |
| if (!c) { |
| return ... |
| } |
| |
| the main path |
| </code></pre></div> |
| |
| <h2 id="design-and-structure">4. Design and Structure</h2> |
| |
| <p>While it is hard to exactly specify what constitutes a good design, there are some properties that can serve as a <em>litmus test</em> for a good design. If these properties are given, the chances are good that the design is going into a good direction. If these properties cannot be achieved, there is a high probability that the design is flawed.</p> |
| |
| <h3 id="immutability-and-eager-initialization">Immutability and Eager Initialization</h3> |
| |
| <ol> |
| <li>Try to use immutable types where possible, especially for APIs, messages, identifiers, properties, configuration, etc.</li> |
| <li>A good general approach is to try and make as many fields of a class <code>final</code> as possible.</li> |
| <li>Classes that are used as keys in maps should be strictly immutable and only have <code>final</code> fields (except maybe auxiliary fields, like lazy cached hash codes).</li> |
| <li>Eagerly initialize classes. There should be no <code>init()</code> or <code>setup()</code> methods. Once the constructor completes, the object should be usable.</li> |
| </ol> |
| |
| <h3 id="nullability-of-the-mutable-parts">Nullability of the Mutable Parts</h3> |
| |
| <p>For nullability, the Flink codebase aims to follow these conventions:</p> |
| |
| <ul> |
| <li>Fields, parameters, and return types are always non-null, unless indicated otherwise</li> |
| <li>All fields, parameters and method types that can be null should be annotated with <code>@javax.annotation.Nullable</code>. |
| That way, you get warnings from IntelliJ about all sections where you have to reason about potential null values.</li> |
| <li>For all mutable (non-final) fields that are not annotated, the assumption is that while the field value changes, there always is a value. |
| <ul> |
| <li>This should be double check whether these can in fact not be null throughout the lifetime of the object.</li> |
| </ul> |
| </li> |
| </ul> |
| |
| <p><em>Note: This means that <code>@Nonnull</code> annotations are usually not necessary, but can be used in certain cases to override a previous annotation, or to point non-nullability out in a context where one would expect a nullable value.</em></p> |
| |
| <p><code>Optional</code> is a good solution as a return type for method that may or may not have a result, so nullable return types are good candidates to be replaced with <code>Optional</code>. |
| See also <a href="code-style-and-quality-java.md#java-optional">usage of Java Optional</a>.</p> |
| |
| <h3 id="avoid-code-duplication">Avoid Code Duplication</h3> |
| |
| <ol> |
| <li>Whenever you are about to copy/paste some code, or reproduce a similar type of functionality in a different place, think about the ways how to refactor/reuse/abstract the changes to avoid the duplication.</li> |
| <li>Common behavior between different specializations should be shared in a common component (or a shared superclass).</li> |
| <li>Always use “private static final” constants instead of duplicating strings or other special values at different locations. Constants should be declared in the top member area of a class.</li> |
| </ol> |
| |
| <h3 id="design-for-testability">Design for Testability</h3> |
| |
| <p>Code that is easily testable typically has good separation of concerns and is structured to be reusable outside the original context (by being easily reusable in tests).</p> |
| |
| <p>A good summary or problems / symptoms and recommended refactoring is in the PDF linked below. |
| Please note that while the examples in the PDF often use a dependency injection framework (Guice), it works in the same way without such a framework.<sup id="fnref:1"><a href="#fn:1" class="footnote">1</a></sup></p> |
| |
| <p><a href="http://misko.hevery.com/attachments/Guide-Writing%20Testable%20Code.pdf">http://misko.hevery.com/attachments/Guide-Writing%20Testable%20Code.pdf</a></p> |
| |
| <p>Here is a compact summary of the most important aspects.</p> |
| |
| <p><strong>Inject dependencies</strong></p> |
| |
| <p>Reusability becomes easier if constructors don’t create their dependencies (the objects assigned to the fields), but accept them as parameters.</p> |
| |
| <ul> |
| <li>Effectively, constructors should have no <code>new</code> keyword.</li> |
| <li>Exceptions are creating a new empty collection (<code>new ArrayList<>()</code>) or similar auxiliary fields (objects that have only primitive dependencies).</li> |
| </ul> |
| |
| <p>To make instantiation easy / readable, add factory methods or additional convenience constructors to construct whole object with dependencies.</p> |
| |
| <p>In no case should it ever be required to use a reflection or a “Whitebox” util to change the fields of an object in a test, or to use PowerMock to intercept a “new” call and supply a mock.</p> |
| |
| <p><strong>Avoid “too many collaborators”</strong></p> |
| |
| <p>If you have to take a big set of other components into account during testing (“too many collaborators”), consider refactoring.</p> |
| |
| <p>The component/class you want to test probably depends on another broad component (and its implementation), rather than on the minimal interface (abstraction) required for its work.</p> |
| |
| <p>In that case, segregate the interfaces (factor out the minimal required interface) and supply a test stub in that case.</p> |
| |
| <ul> |
| <li>For example, if testing a S3RecoverableMultiPartUploader requires actual S3 access |
| then the S3 access should be factored out into an interface and test should replace it by a test stub</li> |
| <li>This naturally requires to be able to inject dependencies (see above)</li> |
| </ul> |
| |
| <p>⇒ Please note that these steps often require more effort in implementing tests (factoring out interfaces, creating dedicated test stubs), but make the tests more resilient to changes in other components, i.e., you do not need to touch the tests when making unrelated changes.</p> |
| |
| <p><strong>Write targeted tests</strong></p> |
| |
| <ul> |
| <li><span style="text-decoration:underline;">Test contracts not implementations</span>: Test that after a sequence of actions, the components are in a certain state, rather than testing that the components followed a sequence of internal state modifications. |
| <ul> |
| <li>For example, a typical antipattern is to check whether one specific method was called as part of the test</li> |
| </ul> |
| </li> |
| <li> |
| <p>A way to enforce this is to try to follow the <em>Arrange</em>, <em>Act</em>, <em>Assert</em> test structure when writing a unit test (<a href="https://xp123.com/articles/3a-arrange-act-assert/">https://xp123.com/articles/3a-arrange-act-assert/</a>)</p> |
| |
| <p>This helps to communicate the intention of the test (what is the scenario under test) rather than the mechanics of the tests. The technical bits go to a static methods at the bottom of the test class.</p> |
| |
| <p>Example of tests in Flink that follow this pattern are:</p> |
| |
| <ul> |
| <li><a href="https://github.com/apache/flink/blob/master/flink-core/src/test/java/org/apache/flink/util/LinkedOptionalMapTest.java">https://github.com/apache/flink/blob/master/flink-core/src/test/java/org/apache/flink/util/LinkedOptionalMapTest.java</a></li> |
| <li><a href="https://github.com/apache/flink/blob/master/flink-filesystems/flink-s3-fs-base/src/test/java/org/apache/flink/fs/s3/common/writer/RecoverableMultiPartUploadImplTest.java">https://github.com/apache/flink/blob/master/flink-filesystems/flink-s3-fs-base/src/test/java/org/apache/flink/fs/s3/common/writer/RecoverableMultiPartUploadImplTest.java</a></li> |
| </ul> |
| </li> |
| </ul> |
| |
| <p><strong>Avoid Mockito - Use reusable test implementations</strong></p> |
| |
| <ul> |
| <li>Mockito-based tests tend to be costly to maintain in the long run by encouraging duplication of functionality and testing for implementation rather than effect |
| <ul> |
| <li>More details: <a href="https://docs.google.com/presentation/d/1fZlTjOJscwmzYadPGl23aui6zopl94Mn5smG-rB0qT8">https://docs.google.com/presentation/d/1fZlTjOJscwmzYadPGl23aui6zopl94Mn5smG-rB0qT8</a></li> |
| </ul> |
| </li> |
| <li>Instead, create reusable test implementations and utilities |
| <ul> |
| <li>That way, when some class changes, we only have to update a few test utils or mocks</li> |
| </ul> |
| </li> |
| </ul> |
| |
| <h3 id="performance-awareness">Performance Awareness</h3> |
| |
| <p>We can conceptually distinguish between code that “coordinates” and code that “processes data”. Code that coordinates should always favor simplicity and cleanness. Data processing code is highly performance critical and should optimize for performance.</p> |
| |
| <p>That means still applying the general idea of the sections above, but possibly forgoing some aspects in some place, in order to achieve higher performance.</p> |
| |
| <p><strong>Which code paths are Data Processing paths?</strong></p> |
| |
| <ul> |
| <li><span style="text-decoration:underline;">Per-record code paths:</span> Methods and code paths that are called for each record. Found for example in Connectors, Serializers, State Backends, Formats, Tasks, Operators, Metrics, runtime data structures, etc.</li> |
| <li><span style="text-decoration:underline;">I/O methods:</span> Transferring messages or chunks of data in buffers. Examples are in the RPC system, Network Stack, FileSystems, Encoders / Decoders, etc.</li> |
| </ul> |
| |
| <p><strong>Things that performance critical code may do that we would otherwise avoid</strong></p> |
| |
| <ul> |
| <li>Using (and reusing) mutable objects to take pressure off the GC (and sometimes help with cache locality), thus forgoing the strive for immutability.</li> |
| <li>Using primitive types, arrays of primitive types, or MemorySegment/ByteBuffer and encoding meaning into the primitive types and byte sequences, rather than encapsulating the behavior in dedicated classes and using objects.</li> |
| <li>Structuring the code to amortize expensive work (allocations, lookups, virtual method calls, …) across multiple records, for example by doing the work once per buffer/bundle/batch.</li> |
| <li>Code layout optimized for the JIT rather than for readability. Examples are inlining fields from other classes (in cases where it is doubtful whether the JIT would do that optimization at runtime), or structuring code to help the JIT compiler with inlining, loop unrolling, vectorization, etc.</li> |
| </ul> |
| |
| <h2 id="concurrency-and-threading">5. Concurrency and Threading</h2> |
| |
| <p><strong>Most code paths should not require any concurrency.</strong> The right internal abstractions should obviate the need for concurrency in almost all cases.</p> |
| |
| <ul> |
| <li>The Flink core and runtime use concurrency to provide these building blocks. |
| Examples are in the RPC system, Network Stack, in the Task’s mailbox model, or some predefined Source / Sink utilities.</li> |
| <li>We are not fully there, but any new addition that introduces implements its own concurrency should be under scrutiny, unless it falls into the above category of core system building blocks.</li> |
| <li>Contributors should reach out to committers if they feel they need to implement concurrent code to see if there is an existing abstraction/building-block, or if one should be added.</li> |
| </ul> |
| |
| <p><strong>When developing a component think about threading model and synchronization points ahead.</strong></p> |
| |
| <ul> |
| <li>For example: single threaded, blocking, non-blocking, synchronous, asynchronous, multi threaded, thread pool, message queues, volatile, synchronized block/methods, mutexes, atomics, callbacks, …</li> |
| <li>Getting those things right and thinking about them ahead is even more important than designing classes interfaces/responsibilities, since it’s much harder to change later on.</li> |
| </ul> |
| |
| <p><strong>Try to avoid using threads all together if possible in any way.</strong></p> |
| |
| <ul> |
| <li>If you feel you have a case for spawning a thread, point this out in the pull request as something to be explicitly reviewed.</li> |
| </ul> |
| |
| <p><strong>Be aware that using threads is in fact much harder than it initially looks</strong></p> |
| |
| <ul> |
| <li>Clean shutdown of threads is very tricky.</li> |
| <li>Handling interruptions in a rock solid fashion (avoid both slow shutdown and live locks) requires almost a Java Wizard</li> |
| <li>Ensuring clean error propagation out of threads in all cases needs thorough design.</li> |
| <li>Complexity of multi-threaded application/component/class grows exponentially, with each additional synchronisation point/block/critical section. Your code initially might be easy enough to understand, but can quickly grow beyond that point.</li> |
| <li>Proper testing of multithreaded code is basically impossible, while alternative approaches (like asynchronous code, non-blocking code, actor model with message queues) are quite easy to test.</li> |
| <li>Usually multi-threaded code is often even less efficient compared to alternative approaches on modern hardware.</li> |
| </ul> |
| |
| <p><strong>Be aware of the java.util.concurrent.CompletableFuture</strong></p> |
| |
| <ul> |
| <li>Like with other concurrent code, there should rarely be the need to use a CompletableFuture</li> |
| <li>Completing a future would also complete on the calling thread any chained futures that are waiting for the result to be completed, unless a completion executor specified explicitly.</li> |
| <li>This can be intentional, if the entire execution should be synchronous / single-threaded, as for example in parts of the Scheduler / ExecutionGraph. |
| <ul> |
| <li>Flink even makes use of a “main-thread executor” to allow calling chained handlers in the same thread as a single-threaded RPC endpoint runs</li> |
| </ul> |
| </li> |
| <li>This can be unexpected, if the thread that completes the future is a sensitive thread. |
| <ul> |
| <li>It may be better to use <code>CompletableFuture.supplyAsync(value, executor)</code> in that case, instead of <code>future.complete(value)</code> when an executor is available</li> |
| </ul> |
| </li> |
| <li>When blocking on a future awaiting completion, always supply a timeout for a result instead of waiting indefinitely, and handle timeouts explicitly.</li> |
| <li>Use <code>CompletableFuture.allOf()</code>/<code>anyOf()</code>, <code>ExecutorCompletionService</code>, or <code>org.apache.flink.runtime.concurrent.FutureUtils#waitForAll</code> if you need to wait for: all the results/any of the results/all the results but handled by (approximate) completion order.</li> |
| </ul> |
| |
| <h2 id="dependencies-and-modules">6. Dependencies and Modules</h2> |
| |
| <ul> |
| <li><strong>Keep the dependency footprint small</strong> |
| <ul> |
| <li>The more dependencies the harder it gets for the community to manage them as a whole.</li> |
| <li>Dependency management includes dependency conflicts, maintaining licenses and related notices, and handling security vulnerabilities.</li> |
| <li>Discuss whether the dependency should be shaded/relocated to avoid future conflicts.</li> |
| </ul> |
| </li> |
| <li><strong>Don’t add a dependency for just one method</strong> |
| <ul> |
| <li>Use Java built-in means if possible.</li> |
| <li>If the method is Apache-licensed, you can copy the method into a Flink utility class with proper attribution.</li> |
| </ul> |
| </li> |
| <li><strong>Declaration of dependencies</strong> |
| <ul> |
| <li>Declare dependencies that you explicitly rely on, whether it provides classes you directly import and use or it’s something that provides a service you directly use, like Log4J.</li> |
| <li>Transitive dependencies should only supply dependencies that are needed at runtime but that you don’t use yourself.</li> |
| <li>[<a href="https://stackoverflow.com/questions/15177661/maven-transitive-dependencies">source</a>]</li> |
| </ul> |
| </li> |
| <li><strong>Location of classes in the Maven modules</strong> |
| <ul> |
| <li>Whenever you create a new class, think about where to put it.</li> |
| <li>A class might be used by multiple modules in the future and might belong into a <code>common</code> module in this case.</li> |
| </ul> |
| </li> |
| </ul> |
| |
| <hr /> |
| |
| <div class="footnotes"> |
| <ol> |
| <li id="fn:1"> |
| |
| <p>We are keeping such frameworks out of Flink, to make debugging easier and avoid dependency clashes. <a href="#fnref:1" class="reversefootnote">↩</a></p> |
| </li> |
| </ol> |
| </div> |
| |
| |
| </div> |
| </div> |
| |
| </div> |
| </div> |
| |
| <hr /> |
| |
| <div class="row"> |
| <div class="footer text-center col-sm-12"> |
| <p>Copyright © 2014-2019 <a href="http://apache.org">The Apache Software Foundation</a>. All Rights Reserved.</p> |
| <p>Apache Flink, Flink®, Apache®, the squirrel logo, and the Apache feather logo are either registered trademarks or trademarks of The Apache Software Foundation.</p> |
| <p><a href="/privacy-policy.html">Privacy Policy</a> · <a href="/blog/feed.xml">RSS feed</a></p> |
| </div> |
| </div> |
| </div><!-- /.container --> |
| |
| <!-- Include all compiled plugins (below), or include individual files as needed --> |
| <script src="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.4/js/bootstrap.min.js"></script> |
| <script src="https://cdnjs.cloudflare.com/ajax/libs/jquery.matchHeight/0.7.0/jquery.matchHeight-min.js"></script> |
| <script src="/js/codetabs.js"></script> |
| <script src="/js/stickysidebar.js"></script> |
| |
| <!-- Google Analytics --> |
| <script> |
| (function(i,s,o,g,r,a,m){i['GoogleAnalyticsObject']=r;i[r]=i[r]||function(){ |
| (i[r].q=i[r].q||[]).push(arguments)},i[r].l=1*new Date();a=s.createElement(o), |
| m=s.getElementsByTagName(o)[0];a.async=1;a.src=g;m.parentNode.insertBefore(a,m) |
| })(window,document,'script','//www.google-analytics.com/analytics.js','ga'); |
| |
| ga('create', 'UA-52545728-1', 'auto'); |
| ga('send', 'pageview'); |
| </script> |
| </body> |
| </html> |
