Change RealtimeIndexTask to use AppenderatorDriver by kevinconaway · Pull Request #5261 · apache/druid

kevinconaway · 2018-01-16T14:58:15Z

@gianm This is a follow up PR to the discussion we had on #4774

This PR changes the RealtimeIndexTask to use the AppenderatorDriver instead of the RealtimePlumber. This allows the realtime indexing to behave similar to the kafka indexing service and ingest events from arbitrary time periods without concerns for windowing.

Implementation notes:

I added a maxRowsPerSegment configuration to RealtimeTuningConfig. I defaulted this to 5 million, the same value that is used in IndexTuningConfig
When maxRowsPerSegment is reached for any segments, all segments are published and handed off in the background. This isn't necessarily ideal because we may end up pushing small segments if there is one large one that reached maxRowsPerSegment

The API of AppenderatorDriver didn't support publishing a single segment so I opted to keep this behavior for now. I suppose this is also the way that the KIS behaves so we are consistent there.
The above behavior means that segments are not merged and pushed on a time-based interval as they were in RealtimePlumber.
The changes I made to TaskLifecyleTest are a little hacky but I wasn't sure of the best way to refactor that test. Suggestions welcome

…ePlumber. Related to apache#4774

jihoonson · 2018-01-16T21:31:46Z

@kevinconaway thanks for the contribution! I'll do my code review shortly. BTW, please check the CI failure (https://travis-ci.org/druid-io/druid/jobs/329503371).

kevinconaway · 2018-01-16T21:42:50Z

Thanks!

I forgot about to run the forbidden API check locally.

kevinconaway · 2018-01-18T16:15:53Z

The build failures here seem unrelated to my commit: https://travis-ci.org/druid-io/druid/builds/329658417?utm_source=github_status&utm_medium=notification

kevinconaway · 2018-01-22T17:38:14Z

@jihoonson will you have time to review this PR this week?

jihoonson · 2018-01-22T17:51:58Z

Sure, I'll review in a few days. In the meantime, please check the CI failure. It looks related to this PR.

…ime-indextask-appenderator

…a race condition where the handoff timeout expires before the segment is published

jihoonson

@kevinconaway I did my first review. Also please fix the conflict.

jihoonson · 2018-01-25T23:41:52Z

    }
  }
+
+  private Appenderator newAppenderator(


Can be static.

I don't mind making these static but will you please help me understand why you prefer them to be static versus instance methods? That will help me conform to your style in the future

Well, I don't have a strong opinion here. It's better for me to read the code because I can expect that any member variables or methods of the class are not used inside in static methods. It's up to you.

jihoonson · 2018-01-25T23:44:33Z

+    );
+  }
+
+  private AppenderatorDriver newDriver(


Can be static.

jihoonson · 2018-01-25T23:47:07Z


  @JsonIgnore
-  private volatile Plumber plumber = null;
+  private final Queue<ListenableFuture<SegmentsAndMetadata>> pendingPublishes;


Should we track all segments in the pendingPublish state? It looks like it would be fine by tracking only the segments in the pendingHandoff state.

I originally had it that way but it introduces a race condition when the user specifies a handoff timeout.

When the user specifies a handoff timeout, its possible that the handoff timeout expires before the segment is published. That was why RealtimeIndexTask#testHandoffTimeout was intermittently failing on travis.

If this isn't a concern, I can refactor that away and modify the test.

Race conditions should be fixed, but I'm not sure where it can exist. Would you elaborate more on this?
What I first thought was keeping all segments in the pendingHandoff state like below.

pendingHandoffs.add( Futures.transform( driver.publish(publisher, committerSupplier.get(), Collections.singletonList(sequenceName)), (AsyncFunction<SegmentsAndMetadata, SegmentsAndMetadata>) segmentsAndMetadata -> driver.registerHandoff(segmentsAndMetadata) ) );

The race condition was in RealtimeIndexTask#testHandoffTimeout. That test expects the handoff to timeout but still have the segments be published.

If you wait on the combined (publish + handoff) future with a timeout, its possible that the timeout expires before the segment is published

jihoonson · 2018-01-25T23:47:55Z

+  private final Queue<ListenableFuture<SegmentsAndMetadata>> pendingHandoffs;
+
+  @JsonIgnore
+  private volatile AppenderatorDriver driver = null;


Can be moved to run() as a local variable.

jihoonson · 2018-01-25T23:59:26Z

+  private final Queue<ListenableFuture<SegmentsAndMetadata>> pendingPublishes;
+
+  @JsonIgnore
+  private final Queue<ListenableFuture<SegmentsAndMetadata>> pendingHandoffs;


If we don't have to keep pendingPublishes, then this can be a List<ListenableFuture<SegmentsAndMetadata>> by letting only the main thread update this list like below.

private void publishSegments( TransactionalSegmentPublisher publisher, Supplier<Committer> committerSupplier, String sequenceName ) { ... pendingHandoffs.add( driver.publishAndRegisterHandoff(publisher, committerSupplier.get(), Collections.singletonList(sequenceName)) ); }

jihoonson · 2018-01-26T00:08:56Z

-              "Cannot acquire a lock for interval[%s]",
-              interval
-          );
+    final VersioningPolicy versioningPolicy = interval -> {


AppenderatorDriver uses an ActionBasedAllocator (https://github.com/druid-io/druid/pull/5261/files#diff-917318f978c8bd3d86decbfdc0166c59R660) which internally acquires a lock for the interval of the allocated segment. If this versioningPolicy is for acquiring a lock, it is not needed.

BTW, I could see that your motivation was to support user-defined versioningPolicy in RealtimeIndexTask (https://groups.google.com/forum/?utm_medium=email&utm_source=footer#!msg/druid-user/AATjuCUFS0E/36OIMaJjAAAJ). This also makes me feel that we need to remove this.

I left the versioningPolicy as it was with the previous incarnation of RealtimeIndexTask, the only change I made was to clean up the anonymous inner class by converting it to a lambda.

I'm happy to remove this if it isn't needed but I didn't feel comfortable removing it without the background on why it was there in the first place

Any Druid's index tasks should acquire a lock for a pair of (dataSource, interval) before writing some data to a segment. This is to prevent multiple tasks from writing for the same interval at the same time.

RealtimeIndexTask was using a VersioningPolicy which is responsible for acquiring a lock for the given interval. This might make sense because the segment version is decided from the acquired lock. However, I feel it's sort of hacky because this VersioningPolicy encompasses some side effects like communicating with overlords.

In addition, this VersioningPolicy internally uses LockAcquireAction which always acquires a lock of the new version. This means, if VersioningPolicy is called for a late arrival, LockAcquireAction returns a new version for the interval of that late arrival, thereby eventually overshadowing all other segments ingested to the same interval so far. SegmentAllocateAction can handle this problem properly and returns a new segmentId for late arrivals which doesn't overshadow other segments ingested by the same task.

BTW, does this PR also targets to support ingestion of late arrivals? I guess it does from this discussion (#4774 (comment)). If so, please add a test for it.

jihoonson · 2018-01-26T00:10:09Z

+    });
+  }
+
+  private String makeSequenceName(int sequenceNumber)


Can be static.

jihoonson · 2018-01-26T00:30:29Z

            final Committer committer = committerSupplier.get();
            final CountDownLatch persistLatch = new CountDownLatch(1);
-            plumber.persist(
+            driver.persist(


Segments should be published and handed off as well as being persisted. driver.publishAndRegisterHandoff() will do all needed things.

Please add a test to check the remaining segments at this point are published well.

I'm not sure thats the correct behavior. From my understanding, this block will only be executed when the task is "stopped gracefully". In that case, I believe the behavior is to persist the state as is with the assumption that the task will be resumed or restarted.

In the happy path case, the segments will have already been persisted. If the task exists with an exception, this block won't get executed.

Thoughts? If my analysis is correct, I can try to make the behavior more explicit in the code

Below is the current behavior:
https://github.com/druid-io/druid/blob/80419752b53b1fe27cc59e6e07a0e8e89cfe5869/indexing-service/src/main/java/io/druid/indexing/common/task/RealtimeIndexTask.java#L444

It only calls plumber.finishJob() if the task was not "gracefully stopped"

Yeah, you're correct. But, why is publishing segments moved from the finally block to the try block? It makes me confused.

himanshug · 2018-01-26T17:13:26Z

@gianm @kevinconaway
I would be more comfortable with this if we left RealtimeIndexTask alone and created a new AppenderatorDriverRealtimeIndexTask and make it configurable to use either at the client side (e.g. Tranquility)
that allows both to coexist at same time and new implementation can be tested in production environments before plumber based RealtimeIndexTask is fully retired.

kevinconaway · 2018-01-26T17:14:53Z

I'm fine with that if that is the direction that you all would like me to go.

jihoonson · 2018-01-26T17:26:45Z

I agree. Actually AppenderatorDriverRealtimeIndexTask has new requirements which were not supported by RealtimeIndexTask. I'm fine by adding a new taskType rather than changing the existing one.

kevinconaway · 2018-01-26T17:29:54Z

Sounds good, I'll update the PR to add a new task type

Actually AppenderatorDriverRealtimeIndexTask has new requirements which were not supported by RealtimeIndexTask.

What do you mean here? Are you talking about the ability to ingest late arriving data and append to existing segments, or something else?

kevinconaway · 2018-01-26T17:47:08Z

Are you OK with this new class sharing the RealtimeIOConfig and RealtimeTuningConfig?

…rt changes to RealtimeIndexTask

…ime-indextask-appenderator

…rt changes to RealtimeIndexTask

jihoonson · 2018-01-26T22:24:45Z

What do you mean here? Are you talking about the ability to ingest late arriving data and append to existing segments, or something else?

@kevinconaway yes, that's exactly what I meant.

Are you OK with this new class sharing the RealtimeIOConfig and RealtimeTuningConfig?

I'm fine if the existing RealtimeIndexTask and the new one share exactly same configurations.

kevinconaway · 2018-01-29T14:49:33Z

I've updated the PR to include a new task, AppenderatorDriverRealtimeIndexTask

I applied the changes that I had made to RealtimeIndexTask and RealtimeIndexTaskTest to this class and its corresponding test case and then reverted the other changes.

jihoonson

Thanks for the quick update. I have some more comments.

windowPeriod, versioningPolicy, rejectionPolicyFactory, persistThreadPriority, and mergeThreadPriority are not used in AppenderatorDriverRealtimeIndexTask. Please log a warning if they are set with AppenderatorDriverRealtimeIndexTask.
RealtimeIndexTask is mainly used by Tranquility. Do you also have a plan to update Tranquility to support this new task type?
Have you had a chance to test in some production environments?

jihoonson · 2018-01-31T00:45:38Z

+        }
+      }
+
+      if (!gracefullyStopped) {


I think this and https://github.com/druid-io/druid/pull/5261/files#diff-43f02f36e4f32ed1d7052b97e9e2fc52R409 are for finalizing the task, and if so, it should be more clear if they are in the same place. I think https://github.com/druid-io/druid/pull/5261/files#diff-43f02f36e4f32ed1d7052b97e9e2fc52R409 doesn't have to be in the finally block and can be moved to here. Also, it would be great for reading code if you can split the long code into some smaller methods.

Also, it would be great for reading code if you can split the long code into some smaller methods.

Sure, I can do that. I had hewed to the same structure and layout as RealtimeIndexTask because I thought that they might be easier to review since you all are probably familiar with how that task works. I'll refactor it.

jihoonson · 2018-01-31T01:11:09Z

+  }
+
+  @Test(timeout = 60_000L)
+  public void testLateData() throws Exception


Would you explain how this tests late arrivals?

Sure. It basically adds two input rows. One "now" and one "now - 2 days". It then tests that both events were added and the data is queryable

firehose.addRows( ImmutableList.of( ImmutableMap.of("t", now.getMillis(), "dim1", "foo", "met1", "1"), // Data is from 2 days ago, should still be processed ImmutableMap.of("t", now.minus(new Period("P2D")).getMillis(), "dim2", "bar", "met1", 2.0) ) ); // .... // Do some queries. Assert.assertEquals(2, sumMetric(task, null, "rows")); Assert.assertEquals(3, sumMetric(task, null, "met1"));

kevinconaway · 2018-01-31T14:40:18Z

windowPeriod, versioningPolicy, rejectionPolicyFactory, persistThreadPriority, and mergeThreadPriority are not used in AppenderatorDriverRealtimeIndexTask. Please log a warning if they are set with AppenderatorDriverRealtimeIndexTask.

👍

RealtimeIndexTask is mainly used by Tranquility. Do you also have a plan to update Tranquility to support this new task type?

No, we don't really use Tranquility so I'm not as familiar with the code base (or Scala for that matter).

Have you had a chance to test in some production environments?

We have been running a previous incarnation of this change (#4774) for months now without any issues. I've been testing this locally and in our staging environment and its performing identically thus far.

jihoonson · 2018-02-01T21:24:23Z

Hi @kevinconaway, would you let me know when you're available for finishing this PR? I'm asking because #5297 is a blocker issue for 0.12.0 which will cause conflicts with this PR. I think it would be easier for me to resolve the conflicts because I reviewed this PR, so I'm waiting for this issue to be merged.

kevinconaway · 2018-02-01T21:26:48Z

I will submit the changes tomorrow, is that OK?

jihoonson · 2018-02-01T21:29:36Z

Yes, it is. Thanks!

…exTask. Combine publish and handoff futures in to single future

kevinconaway · 2018-02-02T01:28:43Z

I've made some readability improvements. I also combined the publish + handoff in to a single future per your request.

Please let me know what else I can change.

Thanks!

jihoonson

@kevinconaway thanks for the quick fix. I left more comments. Please consider them.

jihoonson · 2018-02-02T21:26:13Z

-        }
-      }
-    };
+    final DataSegmentAnnouncer lockingSegmentAnnouncer = createLockingSegmentAnnouncer(toolbox);


You can use the segmentAnnouncer of TaskToolbox. Segment announcing is required to support query processing on realtime data, but similar to the versioningPolicy, locking is not necessary for announcing segments due to the same reason we talked here. KafkaIndexTask is a good example showing how a realtime task acquires locks, allocating and announcing segments, etc.

If I remove the "locking segment announcer", the AppenderatorDriverRealtimeIndexTaskTest#testRestore test fails with:

Caused by: io.druid.java.util.common.ISE: Segments not covered by locks for task: index_realtime_test_ds_0_2018-02-03T00:57:06.929Z_mohhpfgo at io.druid.indexing.common.actions.TaskActionPreconditions.checkLockCoversSegments(TaskActionPreconditions.java:45) ~[classes/:?] at io.druid.indexing.common.actions.SegmentTransactionalInsertAction.perform(SegmentTransactionalInsertAction.java:107) ~[classes/:?] at io.druid.indexing.common.actions.SegmentTransactionalInsertAction.perform(SegmentTransactionalInsertAction.java:47) ~[classes/:?] at io.druid.indexing.common.actions.LocalTaskActionClient.submit(LocalTaskActionClient.java:64) ~[classes/:?] at io.druid.indexing.common.task.AppenderatorDriverRealtimeIndexTask.lambda$run$1(AppenderatorDriverRealtimeIndexTask.java:240) ~[classes/:?] at io.druid.segment.realtime.appenderator.AppenderatorDriver.lambda$publish$139(AppenderatorDriver.java:695) ~[classes/:?] at com.google.common.util.concurrent.Futures$1.apply(Futures.java:713) ~[guava-16.0.1.jar:?] at com.google.common.util.concurrent.Futures$ChainingListenableFuture.run(Futures.java:861) ~[guava-16.0.1.jar:?] ... 3 more

I debugged this a bit and here is what I believe is happening:

On the first run of the task, the task locks are acquired when the segments are allocated via SegmentAllocateAction

The first task is stopped gracefully and the segment identifiers are persisted as the metadata

When the second task runs and restores the metadata, it does not acquire any locks on the restored segment intervals and when the publish happens, the above error happens because there are no locks acquired by the task.

Thoughts? Is that accurate?

I think this can be resolved by borrowing some ideas from KafkaIndexTest

jihoonson · 2018-02-02T21:31:39Z

+    warnIfNotNull(tuningConfig.getWindowPeriod(), "windowPeriod is not used by this task");
+    warnIfNotNull(tuningConfig.getVersioningPolicy(), "versioningPolicy is not used by this task");
+    warnIfNotNull(tuningConfig.getRejectionPolicyFactory(), "rejectionPolicyFactory is not used by this task");
+    warnIfNotNull(tuningConfig.getPersistThreadPriority(), "persistThreadPriority is not used by this task");


tuningConfig.getPersistThreadPriority() returns an integer which cannot be null. This is because this option has a default value.

Probably better to add a new type of tuningConfig which doesn't allow these configurations.

jihoonson · 2018-02-02T21:31:57Z

+    warnIfNotNull(tuningConfig.getVersioningPolicy(), "versioningPolicy is not used by this task");
+    warnIfNotNull(tuningConfig.getRejectionPolicyFactory(), "rejectionPolicyFactory is not used by this task");
+    warnIfNotNull(tuningConfig.getPersistThreadPriority(), "persistThreadPriority is not used by this task");
+    warnIfNotNull(tuningConfig.getMergeThreadPriority(), "mergeThreadPriority is not used by this task");


Same.

Probably better to add a new type of tuningConfig which doesn't allow these configurations.

OK, I'll add a new tuningConfing

jihoonson · 2018-02-02T21:39:29Z

+            AppenderatorDriverAddResult addResult = driver.add(inputRow, sequenceName, committerSupplier);
+
+            if (addResult.isOk()) {
+              if (addResult.isPersistRequired()) {


addResult.isPsersistRequired() always returns false because driver.add(inputRow, sequenceName, committerSupplier) is called which internally allowing automatic persisting.

jihoonson · 2018-02-02T21:53:01Z

+
+
+      if (handoffTimeout > 0) {
+        allHandoffs.get(handoffTimeout, TimeUnit.MILLISECONDS);


Here, futures are actually the tasks publishing as well as handing off segments. So, the timeout should be publishAndHandoffTimeout rather than handoffTimeout. I suggest to add a new tuningConfig type for adding a new configuration which is only used in this task.

…sk. Revert changes to RealtimeTuningConfig

kevinconaway · 2018-02-03T03:29:32Z

I've updated the PR to add a new RealtimeAppenderatorTuningConfig class instead of reusing RealtimeTuningConfig.

jihoonson

@kevinconaway thanks! I left one final comment.

jihoonson · 2018-02-05T18:16:18Z

+import javax.annotation.Nullable;
+import java.io.File;
+
+@JsonTypeName("appenderator")


It would be good if this is synced with the Jackson type name (index_realtime_appenderator) of AppenderatorDriverRealtimeIndexTask.

What do you suggest? realtime_appenderator ?

I suggest index_realtime_appenderator (https://github.com/druid-io/druid/pull/5261/files#diff-df997e60df22e57ef92bff9af75fa9d5R58). This is because users will be easier to remember if they use the same name.
Or I'm ok with changing both type names to realtime_appenderator.

Do you think there is value in keeping the same pattern as the existing realtime task?

As of now, the RealtimeIndexTask task ID is index_realtime with a corresponding tuningConfig value of realtime. If I kept the same pattern for this new task, the ID would be index_realtime_appenderator (as it currently is) and I would change the tuningConfig to be realtime_appenderator.

Thoughts?

Sounds good. It looks better to keep the current pattern.

No problem, updated.

Thanks for the quick fix!

…e naming pattern as RealtimeIndexTask

jihoonson

+1 after CI.

kevinconaway · 2018-02-06T01:32:43Z

@jihoonson Not sure why this particular build failed, it seems unrelated

jihoonson · 2018-02-06T01:38:06Z

@kevinconaway just restarted the failed job.

kevinconaway · 2018-02-06T01:53:51Z

@jihoonson Is this particular test known to be flaky?

Are you able to get the test output off the build nodes?

kevinconaway · 2018-02-06T02:20:05Z

Looks like it failed here and here too

jihoonson · 2018-02-06T05:12:16Z

@kevinconaway yeah, looks like flaky. The failed task passes in my mac.
Reopened #2373

kevinconaway · 2018-02-06T13:26:03Z

OK thanks. What are the next steps for this PR?

jihoonson · 2018-02-06T17:45:27Z

I think it's ready for merging. @himanshug do you have further comments?

himanshug · 2018-02-06T18:09:55Z

@jihoonson I'm sorry but I haven't really looked at the patch. Given that it is a new task now that can be tested independently in prod and wouldn't impact existing users. I'm fine with your approval on the PR. Feel free to merge.

jihoonson · 2018-02-06T18:12:21Z

@himanshug thanks. I'll merge shortly.

jihoonson · 2018-02-06T19:17:02Z

@kevinconaway raised #5355. Please take a look.

kevinconaway · 2018-02-20T15:37:06Z

@jihoonson Thoughts on backporting this to 0.12?

jihoonson · 2018-02-20T18:10:49Z

@kevinconaway basically, I think we don't have to backport this to 0.12 because this is a new feature rather than improving an existing one. However, I think it would be fine if this doesn't cause a lot of conflicts for backporting. More people can test this new task type earlier instead of waiting for a next release or backporting this by themselves. @gianm @jon-wei what do you think?

kevinconaway · 2018-02-20T18:13:29Z

basically, I think we don't have to backport this to 0.12 because this is a new feature rather than improving an existing one.

The reason I ask is that I'd like to get off our fork and start using this feature when 0.12 is released, rather than waiting for 0.13

jihoonson · 2018-02-20T18:39:05Z

@kevinconaway I'm sorry to say, but we usually backports only bug fixes, so I would say it's difficult to make an exception for this PR without a reasonable reason. We already created 0.12.0-rc3 (https://groups.google.com/forum/#!topic/druid-development/DZx-opThc50) which will be released soon unless another bug is found.

kevinconaway · 2018-02-20T18:40:22Z

No worries, thanks for following up

jihoonson · 2018-02-20T18:47:15Z

Thanks for understanding!

dclim · 2018-02-21T00:16:10Z

@kevinconaway are you a Tranquility user? I'm curious whether you've looked into modifying Tranquility to make use of this appenderator-based realtime task

kevinconaway · 2018-02-21T02:57:05Z

@dclim We're not using Tranquility. I don't know Scala well enough to confidently hack on it. The last time I looked at it, it wasn't up to date with even Druid 0.10 but that was some time ago

dclim · 2018-02-21T05:51:33Z

@kevinconaway cool, thank you for the contribution!

Kevin Conaway added 2 commits January 15, 2018 16:48

Change RealtimeIndexTask to use AppenderatorDriver instead of Realtim…

f774d3c

…ePlumber. Related to apache#4774

Remove unused throwableDuringPublishing

5d9487b

kevinconaway mentioned this pull request Jan 16, 2018

Change RealtimePlumber to use SegmentAllocate API #4774

Closed

Fix usage of forbidden API

ce0b22c

Kevin Conaway added 3 commits January 22, 2018 13:01

Merge branch 'master' of https://github.com/druid-io/druid into realt…

5fe7b84

…ime-indextask-appenderator

Update realtime index IT to account for not skipping older data any more

f43d922

Separate out waiting on publish futures and handoff futures to avoid …

55af8be

…a race condition where the handoff timeout expires before the segment is published

jihoonson reviewed Jan 26, 2018

View reviewed changes

Kevin Conaway added 3 commits January 26, 2018 16:31

apache#5261 Add separate AppenderatorDriverRealtimeIndexTask and reve…

9ae8c21

…rt changes to RealtimeIndexTask

Merge branch 'master' of https://github.com/druid-io/druid into realt…

a86e11b

…ime-indextask-appenderator

apache#5261 Add separate AppenderatorDriverRealtimeIndexTask and reve…

cdeac02

…rt changes to RealtimeIndexTask

jihoonson mentioned this pull request Jan 31, 2018

Change early publishing to early pushing in indexTask & refactor AppenderatorDriver #5297

Merged

jihoonson reviewed Jan 31, 2018

View reviewed changes

apache#5261 Readability improvements in AppenderatorDriverRealtimeInd…

bc927c2

…exTask. Combine publish and handoff futures in to single future

jihoonson reviewed Feb 2, 2018

View reviewed changes

apache#5261 Add separate tuningConfig for RealtimeAppenderatorIndexTa…

764d97e

…sk. Revert changes to RealtimeTuningConfig

jihoonson reviewed Feb 5, 2018

View reviewed changes

apache#5261 Change JSON type to realtime_appenderator to keep the sam…

cba0c4e

…e naming pattern as RealtimeIndexTask

jihoonson approved these changes Feb 6, 2018

View reviewed changes

jihoonson merged commit 93fdbcb into apache:master Feb 6, 2018

jihoonson mentioned this pull request Feb 6, 2018

Integration test for AppenderatorDriverRealtimeIndexTask #5355

Merged

dclim added this to the 0.13.0 milestone Oct 8, 2018



		if (handoffTimeout > 0) {
		allHandoffs.get(handoffTimeout, TimeUnit.MILLISECONDS);

Conversation

kevinconaway commented Jan 16, 2018

Uh oh!

jihoonson commented Jan 16, 2018

Uh oh!

kevinconaway commented Jan 16, 2018

Uh oh!

kevinconaway commented Jan 18, 2018

Uh oh!

kevinconaway commented Jan 22, 2018

Uh oh!

jihoonson commented Jan 22, 2018

Uh oh!

jihoonson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kevinconaway Jan 26, 2018 • 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

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!

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!

himanshug commented Jan 26, 2018

Uh oh!

kevinconaway commented Jan 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jihoonson commented Jan 26, 2018

Uh oh!

kevinconaway commented Jan 26, 2018

Uh oh!

kevinconaway commented Jan 26, 2018

Uh oh!

jihoonson commented Jan 26, 2018

Uh oh!

kevinconaway commented Jan 29, 2018

Uh oh!

jihoonson left a comment

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!

kevinconaway Jan 26, 2018 •

edited

Loading

kevinconaway commented Jan 26, 2018 •

edited

Loading