[FLINK-12555] Introduce an encapsulated metric group layout for shuffle API #8485

azagrebin · 2019-05-20T09:28:15Z

What is the purpose of the change

At the moment, partition/gate create methods in NetworkEnvironment have a lot of metrics arguments to maintain original layout for metric groups. This approach is not quite encapsulated and clean for shuffle API. We can have just one parent group for shuffle metrics. The old layout can be still maintained in parallel and deprecated. At the moment we can do it with a couple of casts (if shuffle implementation is NetworkEnvironment) and adding an additional legacy metric registration which can be removed later.

Brief change log

Change NetworkEnvironment.createResultPartitionWriters/createInputGates to have only one parent metric group argument.
Add InputChannelMetricsWithLegacy to increment input metrics from the legacy group as well
Move legacy metric group creation to deprecated NetworkEnvironment.registerLegacyNetworkMetrics
clean Task of legacy code but add call to NetworkEnvironment.registerLegacyNetworkMetrics after creation of partitions and gates (later needs instanceOf check for actual ShuffleService)

Verifying this change

simple refactoring, existing tests

Does this pull request potentially affect one of the following parts:

Dependencies (does it add or upgrade a dependency): (no)
The public API, i.e., is any changed class annotated with @Public(Evolving): (yes)
The serializers: (no)
The runtime per-record code paths (performance sensitive): (no)
Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (no)
The S3 file system connector: (no)

Documentation

Does this pull request introduce a new feature? (yes / no)
If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)

azagrebin · 2019-05-20T09:29:12Z

The new metric group layout is still to discuss.
We also need to agree on how we deprecate the old groups in docs.

flinkbot · 2019-05-20T09:30:28Z

Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community
to review your pull request. We will use this comment to track the progress of the review.

Review Progress

❓ 1. The [description] looks good.
❓ 2. There is [consensus] that the contribution should go into to Flink.
❗ 3. Needs [attention] from.
- Needs attention by @zentol [PMC], @zhijiangW
❓ 4. The change fits into the overall [architecture].
❓ 5. Overall code [quality] is good.

Please see the Pull Request Review Guide for a full explanation of the review process.

Details

The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required

Bot commands

The @flinkbot bot supports the following commands:

@flinkbot approve description to approve one or more aspects (aspects: description, consensus, architecture and quality)
@flinkbot approve all to approve all aspects
@flinkbot approve-until architecture to approve everything until architecture
@flinkbot attention @username1 [@username2 ..] to require somebody's attention
@flinkbot disapprove architecture to remove an approval you gave earlier

azagrebin · 2019-05-20T09:37:54Z

@flinkbot attention @zentol @zhijiangW

zentol

As for the deprecation in the documentation, could you write down a list of all metrics that are being modified, with the original and modified scope?

flink-runtime/src/main/java/org/apache/flink/runtime/taskmanager/Task.java

zentol · 2019-05-23T12:34:46Z

...k-runtime/src/main/java/org/apache/flink/runtime/io/network/metrics/InputChannelMetrics.java

+
+		@Override
+		public void incNumBytesInLocalCounter(long inc) {
+			super.incNumBytesInLocalCounter(inc);


it's quite easy to encapsulate this logic in a Counter implementation, which would require significantly less overall changes and would no longer introduce a new special case on how metrics are being used.

public static class CounterWrapper implements Counter { private final Counter counter1; private final Counter counter2; private CounterWrapper(Counter counter1, Counter counter2) { this.counter1 = counter1; this.counter2 = counter2; } @Override public void inc() { counter1.inc(); counter2.inc(); } @Override public void inc(long n) { counter1.inc(n); counter2.inc(n); } @Override public void dec() { counter1.dec(); counter2.dec(); } @Override public void dec(long n) { counter1.dec(n); counter2.dec(n); } @Override public long getCount() { // assume that the counters are not accessed directly elsewhere return counter1.getCount(); } }

true, switched to the counter wrapper

azagrebin · 2019-05-23T13:42:15Z

Thanks @zentol ! addressed comments

zhijiangW · 2019-05-27T02:31:12Z

...k-runtime/src/main/java/org/apache/flink/runtime/io/network/metrics/InputChannelMetrics.java

+
+		@Override
+		public long getCount() {
+			// assume that the counters are not accessed directly elsewhere


I am not sure whether getCounter would be actually used. If used we should keep the return counter as previous structure. That means calling `new InputChannelMetrics(parentGroup, networkGroup) instead?

zhijiangW

Thanks for opening this PR @azagrebin !

TBH I also do not like the way of involving in multiple metric related parameters when introducing ShuffleService#createResultPartitionWriters/InputGates, and we also talked about this issue before. My previous thought and way was also providing only a parent metric in creation methods, and make create Network and buffers groups inside createResultPartitionWriters/InputGates . But the concerns were that the metric groups are created more than once which was not consistent with previous way and this is also relied on the internal implementations of metric framework, so we gave up that way then.

But in this PR we still create the Network metric group twice which stills the same as my previous thought. I am not very clear what is the new thoughts behind this?

Actually there are two issues in this PR:

One is avoiding multiple metric parameters in creation of partition/gate. And if we think there are no concerns for creating one metric group twice, we could only pass the metrics.getIOMetricGroup() to replace all, and create the buffers and Network groups twice inside creatResultPartitionWriters/InputGates separately. The previous metric structure is no need to change for this motivation.
Another is we want to change the previous metric structure for shuffle service, that means make previous buffers group under the Network group. I agree with this point which seems more clearly within the same root parent architecture.

I think it is better to make the first issue as a separate hotfix commit.

zentol · 2019-05-27T13:28:43Z

And if we think there are no concerns for creating one metric group twice

There are concerns; it prints a warning to the user as it is not how the API is supposed to be used. If any components need access to a shared group then this group should be created once and passed around as needed.
Things were implemented this way on purpose to prevent components from interfering with each other by accident.

zhijiangW · 2019-05-27T14:31:25Z

Thanks for the replies @zentol .

But in this PR parentGroup.addGroup("Network") is called twice in NetworkEnvironment#createResultPartitionWriters/InputGates. So we should change to create it still in task class and then pass it into createResultPartitionWriters/InputGates separately?

azagrebin · 2019-05-27T19:05:50Z

True, thanks for noticing it @zhijiangW, it was not an intent. I will change it to create group only once. It can be called e.g. NetworkInput/NetworkOutput.

azagrebin · 2019-06-03T15:33:07Z

@zentol
I have updated the docs.
Here are the moved metrics:

Deprecated				New
Type	Scope	Infix	Name	Type	Scope	Infix	Name
IO	Task	-	numBytesInLocal	Default shuffle service	Task	Shuffle.Netty.Input	numBytesInLocal
IO	Task	-	numBytesInLocalPerSecond	Default shuffle service	Task	Shuffle.Netty.Input	numBytesInLocalPerSecond
IO	Task	-	numBytesInRemote	Default shuffle service	Task	Shuffle.Netty.Input	numBytesInRemote
IO	Task	-	numBytesInRemotePerSecond	Default shuffle service	Task	Shuffle.Netty.Input	numBytesInRemotePerSecond
IO	Task	-	numBuffersInLocal	Default shuffle service	Task	Shuffle.Netty.Input	numBuffersInLocal
IO	Task	-	numBuffersInLocalPerSecond	Default shuffle service	Task	Shuffle.Netty.Input	numBuffersInLocalPerSecond
IO	Task	-	numBuffersInRemote	Default shuffle service	Task	Shuffle.Netty.Input	numBuffersInRemote
IO	Task	-	numBuffersInRemotePerSecond	Default shuffle service	Task	Shuffle.Netty.Input	numBuffersInRemotePerSecond
Network	TaskManager	Status.Network	AvailableMemorySegments	Default shuffle service	TaskManager	Status.Shuffle.Netty	AvailableMemorySegments
Network	TaskManager	Status.Network	TotalMemorySegments	Default shuffle service	TaskManager	Status.Shuffle.Netty	TotalMemorySegments
Network	Task	Network.<Input\|Output>.<gate\|partition>	totalQueueLen	Default shuffle service	Task	Shuffle.Netty.<Input\|Output>.<gate\|partition>	totalQueueLen
Network	Task	Network.<Input\|Output>.<gate\|partition>	minQueueLen	Default shuffle service	Task	Shuffle.Netty.<Input\|Output>.<gate\|partition>	minQueueLen
Network	Task	Network.<Input\|Output>.<gate\|partition>	maxQueueLen	Default shuffle service	Task	Shuffle.Netty.<Input\|Output>.<gate\|partition>	maxQueueLen
Network	Task	Network.<Input\|Output>.<gate\|partition>	avgQueueLen	Default shuffle service	Task	Shuffle.Netty.<Input\|Output>.<gate\|partition>	avgQueueLen
Network	Task	buffers	inputQueueLength	Default shuffle service	Task	Shuffle.Netty.Input.Buffers	inputQueueLength
Network	Task	buffers	outputQueueLength	Default shuffle service	Task	Shuffle.Netty.Output.Buffers	outputQueueLength
Network	Task	buffers	inPoolUsage	Default shuffle service	Task	Shuffle.Netty.Input.Buffers	inPoolUsage
Network	Task	buffers	outPoolUsage	Default shuffle service	Task	Shuffle.Netty.Output.Buffers	outPoolUsage

zentol · 2019-06-05T09:48:46Z

I'd prefer if the infix were Shuffle.Netty.Input/Shuffle.Netty.Output, so far we've followed a hierarchical style similarly to out config options.

The PR also needs a rebase.

zentol · 2019-06-05T09:49:51Z

docs/monitoring/metrics.md

      <th rowspan="12"><strong>Task</strong></th>
      <td>numBytesInLocal</td>
-      <td>The total number of bytes this task has read from a local source.</td>
+      <td>The total number of bytes this task has read from a local source. Deprecated: see NettyShuffle.</td>


let's remove the description entirely and put a nice attention icon before the deprecation notice.

zentol · 2019-06-05T09:51:05Z

docs/monitoring/metrics.md

  </tbody>
 </table>

+### NettyShuffle


do users actually know what NettyShuffle is?

True users do not know about NettyShuffle, we will still have only one at the moment.
It could be Default network shuffle then.

zhijiangW · 2019-06-06T03:24:28Z

flink-runtime/src/main/java/org/apache/flink/runtime/taskmanager/Task.java

-		final MetricGroup networkGroup = metrics.getIOMetricGroup().addGroup("Network");
-		final MetricGroup outputGroup = networkGroup.addGroup("Output");
-		final MetricGroup inputGroup = networkGroup.addGroup("Input");
+		MetricGroup taskShuffleMetricGroup = networkEnvironment.createTaskMetricGroup(metrics.getIOMetricGroup());


Does it mean ShuffleEnvironment interface need provide the method createTaskMetricGroup?

My idea was to expose TaskShuffleContext ShuffleEnvironment.createTaskShuffleContext(taskName, execId, parentMetricGroup) in ShuffleEnvironment . TaskShuffleContext would contain taskName, execId and created in/output metric group as well. createPartition/Gate methods would take the TaskShuffleContext. This way ShuffleEnvironment can create its own metric group once and other task common parameters are encapsulated.

zhijiangW · 2019-06-06T03:49:26Z

flink-runtime/src/main/java/org/apache/flink/runtime/io/network/NetworkEnvironment.java

+		// similar to MetricUtils.instantiateNetworkMetrics() but inside this IOMetricGroup
+		final MetricGroup networkGroup = metricGroup.addGroup("Network");
+		final MetricGroup outputGroup = networkGroup.addGroup("Output");
+		final MetricGroup inputGroup = networkGroup.addGroup("Input");


Use METRIC_GROUP_OUTPUT and METRIC_GROUP_INPUT instead of Output and Input, also might define a class var for Network.

zhijiangW · 2019-06-06T03:49:50Z

flink-runtime/src/main/java/org/apache/flink/runtime/io/network/NetworkEnvironment.java


-	private static final String METRIC_GROUP_NETWORK = "Network";
+	@SuppressWarnings("DeprecatedIsStillUsed")
+	@Deprecated private static final String METRIC_GROUP_NETWORK_DEPRECATED = "Network";


Put @Deprecated in separate line?

zhijiangW · 2019-06-06T08:35:08Z

...ime/src/main/java/org/apache/flink/runtime/io/network/metrics/NettyShuffleMetricFactory.java

+	private static final String METRIC_INPUT_QUEUE_LENGTH = "inputQueueLength";
+	private static final String METRIC_INPUT_POOL_USAGE = "inPoolUsage";
+
+	private NettyShuffleMetricFactory() {


This constructor seems not necessary

This is an indicator that it is a utility stateless class with only static methods which is not supposed to be instantiated.

zhijiangW · 2019-06-06T08:38:30Z

Thanks for the updates @azagrebin !
I like the way of metric factory to make NetworkEnvironment simple. Only several nit comments.

zentol · 2019-06-06T09:56:04Z

docs/monitoring/metrics.md

+      <td>Gauge</td>
+    </tr>
+    <tr>
+      <td rowspan="2">Shuffle.Netty.Output.buffers</td>


buffers is lower-case here, but upper-case above for input

zentol · 2019-06-07T08:33:11Z

flink-runtime/src/main/java/org/apache/flink/runtime/taskmanager/Task.java


+		// we will have to check type of shuffle service later whether it is NetworkEnvironment
+		//noinspection deprecation
+		networkEnvironment.registerLegacyNetworkMetrics(metrics.getIOMetricGroup(), resultPartitionWriters, gates);


Since we intend to remove this in the next release It should be fine to use a cast here, so that we don't have to add such a short-lived method to the interface.

azagrebin · 2019-06-07T14:46:57Z

Thanks for the reviews @zentol @zhijiangW, I've addressed comments
This PR is now based on #8608 which I think needs to be merged first.
While rebasing, I introduced a ShuffleIOOwnerContext which is now created by Task using ShuffleEnvironment.createShuffleIOOwnerContext before creating partitions/gates. NettyShuffleEnvironment.createShuffleIOOwnerContext also creates netty metric groups, only once.

zhijiangW · 2019-06-10T07:39:24Z

flink-runtime/src/main/java/org/apache/flink/runtime/taskmanager/Task.java

-		final MetricGroup outputGroup = networkGroup.addGroup("Output");
-		final MetricGroup inputGroup = networkGroup.addGroup("Input");
+		ShuffleIOOwnerContext taskShuffleContext = shuffleEnvironment
+			.createShuffleIOOwnerContext(taskNameWithSubtaskAndId, executionId, metrics.getIOMetricGroup());


The formatting : shuffleEnvironment.createShuffleIOOwnerContext in one line and make every parameter in separate line?

In this case we do not break the arg list, the whole function is on the new line, same as chained calls.

zhijiangW · 2019-06-10T07:49:00Z

flink-runtime/src/main/java/org/apache/flink/runtime/taskmanager/Task.java

+			.createShuffleIOOwnerContext(taskNameWithSubtaskAndId, executionId, metrics.getIOMetricGroup());

 		// produced intermediate result partitions
 		final ResultPartitionWriter[] resultPartitionWriters = shuffleEnvironment.createResultPartitionWriters(


remove final for resultPartitionWriters for consistent with other temp vars?

this file actually looks like more using finals atm.

But the vars of taskShuffleContext and following gates are not defined as final. ATM only taskNameWithSubtaskAndId and resultPartitionWriters are final. Or we could make the new added taskShuffleContext as final to keep consistency.

zhijiangW · 2019-06-10T07:50:24Z

flink-runtime/src/main/java/org/apache/flink/runtime/taskmanager/Task.java


 		// create the reader and writer structures

 		final String taskNameWithSubtaskAndId = taskNameWithSubtask + " (" + executionId + ')';


Might remove this local var to use taskNameWithSubtask + " (" + executionId + ')' directly below.

I would keep it to not mix concerns in one line.

I could agree your way to keep it. :)

My previous concern is that taskNameWithSubtaskAndId was used in many places before, so it worths defining a local var. Now it is only used in one place and we could reduce one line to make task constructor short to remove it.
Another small consideration is avoiding final issue as above mentioned.

zhijiangW · 2019-06-10T07:58:07Z

flink-runtime/src/main/java/org/apache/flink/runtime/shuffle/ShuffleEnvironment.java

+	 * <p>This method has to be called only once to avoid duplicated internal metric group registration.
+	 *
+	 * @param ownerName the owner name, used for logs
+	 * @param executionAttemptID execution attempt id of the producer or consumer


of the producer or consumer -> of the owner?

At the moment we have exec id only for tasks/executions, in this context producer or consumer. I thought about owner as a potentially any shuffle IO component within task/execution, possibly even source/sink

zhijiangW · 2019-06-10T08:42:38Z

flink-runtime/src/main/java/org/apache/flink/runtime/io/network/NettyShuffleEnvironment.java

+			checkNotNull(ownerName),
+			checkNotNull(executionAttemptID),
+			parentGroup,
+			nettyGroup.addGroup(METRIC_GROUP_INPUT),


It might be better to put these two addGroup covered by above createShuffleIOOwnerMetricGroup then return tuple2 to reference here because all the metrics related operations should be done inside NettyShuffleMetricFactory

I thought about it, I think the operation is too primitive atm for an extra utility method. Besides, I think that in general tuple2 is very adhoc and not so readable.

Actually I also do not like the way of tuple.
But we provide the method of createShuffleIOOwnerMetricGroup in factory, and it is only used for adding group simple. So it might bring confused that which addGroup should be done inside factory and which should be done outside. The scope/rule seems not clear.

zhijiangW · 2019-06-10T08:51:59Z

flink-runtime/src/main/java/org/apache/flink/runtime/io/network/NettyShuffleEnvironment.java

+			registerOutputMetrics(
+				config.isNetworkDetailedMetrics(),
+				outputMetricGroup,
+				outputMetricGroup.addGroup(METRIC_GROUP_BUFFERS),


Might be better to cover all the metric related details in NettyShuffleMetricFactory. I mean the logic of outputMetricGroup.addGroup(METRIC_GROUP_BUFFERS) here.

I can add overloaded methods for this in NettyShuffleMetricFactory.

zhijiangW · 2019-06-10T09:00:55Z

flink-runtime/src/main/java/org/apache/flink/runtime/io/network/NettyShuffleEnvironment.java

+
+			MetricGroup networkInputGroup = ownerContext.getInputGroup();
+			@SuppressWarnings("deprecation")
+			InputChannelMetrics inputChannelMetrics = new InputChannelMetrics(networkInputGroup, ownerContext.getParentGroup());


Maybe we could delay the creation of InputChannelMetrics in the SingleInputGateFactory#createInputChannels, then the following would be
SingleInputGate inputGate = singleInputGateFactory.create(ownerContext, icdd, partitionProducerStateProvider) instead.
To do so we could make the current createInputGates simple and make all the things done when required.

The gate creation loop will create InputChannelMetrics multiple times, I would avoid this because InputChannelMetrics are shared by gates at the moment.

zhijiangW · 2019-06-10T09:01:13Z

flink-runtime/src/main/java/org/apache/flink/runtime/io/network/NettyShuffleEnvironment.java

+			registerInputMetrics(
+				config.isNetworkDetailedMetrics(),
+				networkInputGroup,
+				networkInputGroup.addGroup(METRIC_GROUP_BUFFERS),


ditto: for the issue of networkInputGroup.addGroup(METRIC_GROUP_BUFFERS)

zhijiangW · 2019-06-10T09:02:47Z

Thanks for the updates @azagrebin .
I left some final nit comments relevant with new introduced context.

azagrebin · 2019-06-10T18:25:54Z

Thanks @zhijiangW , I have addressed comments

zentol · 2019-06-14T10:14:53Z

flink-runtime/src/main/java/org/apache/flink/runtime/shuffle/ShuffleEnvironment.java

+	 * @param parentGroup parent of shuffle specific metric group
+	 * @return context of the shuffle input/output owner used to create partitions or gates belonging to the owner
+	 */
+	ShuffleIOOwnerContext createShuffleIOOwnerContext(


it would be useful the partition lifecycle management if this method would also accept the JobID..

Shuffle service does not need it at the moment, would be nice actually to keep it decoupled from the job id, I would suggest we discuss it at the corresponding partition lifecycle PR and add it if needed.

see this discussion then: #8687 (comment)

zentol · 2019-06-14T10:16:29Z

flink-runtime/src/main/java/org/apache/flink/runtime/shuffle/ShuffleEnvironment.java

+	 * @param resultPartitionDeploymentDescriptors descriptors of the partition, produced by the owner
+	 * @return collection of the {@link ResultPartitionWriter ResultPartitionWriters}
+	 */
+	Collection<P> createResultPartitionWriters(


have you considered moving this method and createInputGates into the ShuffleIOOwnerContext, and only pass this into the task instead of the entire ShuffleEnvironment? The task only requires access to these 2 methods, while any other users of the ShuffleEnvironment doesn't need these.

This is an interesting idea, we can consider it as a follow-up issue.

azagrebin · 2019-06-17T09:28:29Z

Rebased on #8680

…le API and deprecate old one

…metrics creation

rmetzger added the review=description? label May 20, 2019

rmetzger requested a review from zentol May 20, 2019 09:38

rmetzger added the component=Runtime/Network label May 20, 2019

azagrebin changed the title ~~[FLINK-12555] Introduce an incapsulated metric group layout for shuffle API~~ [FLINK-12555] Introduce an encapsulated metric group layout for shuffle API May 20, 2019

zentol self-assigned this May 22, 2019

azagrebin force-pushed the FLINK-12555 branch from df46a44 to a4ad533 Compare May 22, 2019 13:09

zentol reviewed May 23, 2019

View reviewed changes

rmetzger requested a review from zentol May 23, 2019 12:48

azagrebin force-pushed the FLINK-12555 branch from 4df5e8b to 62b1bb3 Compare May 23, 2019 13:40

zhijiangW reviewed May 27, 2019

View reviewed changes

azagrebin force-pushed the FLINK-12555 branch from 62b1bb3 to 769a0f7 Compare June 3, 2019 13:31

zentol reviewed Jun 5, 2019

View reviewed changes

rmetzger requested a review from zentol June 5, 2019 09:50

zentol reviewed Jun 5, 2019

View reviewed changes

rmetzger requested a review from zentol June 5, 2019 09:52

azagrebin force-pushed the FLINK-12555 branch 2 times, most recently from 134fef6 to 97144b2 Compare June 5, 2019 14:12

zhijiangW reviewed Jun 6, 2019

View reviewed changes

azagrebin force-pushed the FLINK-12555 branch 2 times, most recently from 083b0e4 to 46901b2 Compare June 6, 2019 11:06

zentol reviewed Jun 7, 2019

View reviewed changes

azagrebin force-pushed the FLINK-12555 branch from 46901b2 to 5c6263e Compare June 7, 2019 14:39

azagrebin force-pushed the FLINK-12555 branch from 5c6263e to ed5fa08 Compare June 7, 2019 14:47

zhijiangW reviewed Jun 10, 2019

View reviewed changes

azagrebin force-pushed the FLINK-12555 branch 2 times, most recently from b23888c to 8f13b9d Compare June 10, 2019 13:02

zhijiangW mentioned this pull request Jun 12, 2019

[FLINK-12612][coordination] Track stored partition on the TaskExecutor #8687

Merged

zentol reviewed Jun 14, 2019

View reviewed changes

azagrebin force-pushed the FLINK-12555 branch from 8f13b9d to e85f42d Compare June 17, 2019 09:27

zhijiangW mentioned this pull request Jun 19, 2019

[FLINK-12284][Network,Metrics]Fix the incorrect inputBufferUsage metric in credit-based network mode #8455

Merged

azagrebin force-pushed the FLINK-12555 branch from e85f42d to 2e2c257 Compare June 20, 2019 08:40

azagrebin added 2 commits June 21, 2019 14:57

[FLINK-12555] Introduce an encapsulated metric group layout for shuff…

8156769

…le API and deprecate old one

[hotfix][network] Introduce NettyShuffleMetricFactory to encapsulate …

92d50ff

…metrics creation

azagrebin force-pushed the FLINK-12555 branch from 2e2c257 to 92d50ff Compare June 21, 2019 12:58

zentol merged commit 3c558d8 into apache:master Jun 24, 2019


		// create the reader and writer structures

		final String taskNameWithSubtaskAndId = taskNameWithSubtask + " (" + executionId + ')';

[FLINK-12555] Introduce an encapsulated metric group layout for shuffle API #8485

[FLINK-12555] Introduce an encapsulated metric group layout for shuffle API #8485

Uh oh!

Conversation

azagrebin commented May 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What is the purpose of the change

Brief change log

Verifying this change

Does this pull request potentially affect one of the following parts:

Documentation

Uh oh!

azagrebin commented May 20, 2019

Uh oh!

flinkbot commented May 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review Progress

Uh oh!

azagrebin commented May 20, 2019

Uh oh!

zentol left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

azagrebin commented May 23, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhijiangW left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zentol commented May 27, 2019

Uh oh!

zhijiangW commented May 27, 2019

Uh oh!

azagrebin commented May 27, 2019

Uh oh!

azagrebin commented Jun 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zentol commented Jun 5, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhijiangW commented Jun 6, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

azagrebin commented Jun 7, 2019

Uh oh!

zhijiangW Jun 10, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

azagrebin commented May 20, 2019 •

edited

Loading

flinkbot commented May 20, 2019 •

edited

Loading

zhijiangW left a comment •

edited

Loading

azagrebin commented Jun 3, 2019 •

edited

Loading

zhijiangW Jun 10, 2019 •

edited

Loading

zhijiangW Jun 10, 2019 •

edited

Loading