[NEMO-7] Intra-TaskGroup pipelining #2

jeongyooneo · 2018-03-05T10:08:21Z

JIRA: NEMO-7: Refactor TaskGroupExecutor to a batch/stream-unified execution model

Major changes:

Refactored TaskGroupExecutor to a pipelined execution model(Thanks to @sanha)
- Per-Task InputReader/OutputWriter and TaskState management are removed.
- Per-Task Pipe acts as a local writer for this Task to where it emits output, and a local reader for its children Tasks from where they receives input.
- Input is deserialized element-wise, trickle down to the end of TaskGroup DAG in a recursive manner.
  - iteratorIdToDstTasksMap maintains source input(data from bounded source or other TaskGroups) to the Task DAG that processes this input.
  - pipeToDstTasksMap maintains intra-TaskGroup input to the Task DAG that processes this input.

TaskGroup-wise InputReaders and OutputWriters

Recursive execution of intra-TaskGroup pipelining

Minor changes to note:

None

Tests for the changes:

The existing tests are sufficient.

Other comments:

None

Closes #268 #268 has reported that importing some Beam's hdfs-related maven artifacts causes integration failures. But I checked that it causes no problem and the tests run well. So now we can use it to deal with IO to/from Hdfs.

Resolves #235. - On master side, removed the Task state updating which isn't used anywhere. - On master and executor side, removed failedTaskIds information which is not used. The only Task state that is meaningful to master is ON_HOLD, thus tasksPutOnHold information is maintained.

* [Vortex-418] Collect Executor Metric in Master * Flush remaining metrics while closing the metric sender * rm index increment

This PR addressed #404 by - implements MetadataManager that manages the metadata in the master side - makes RemoteFileMetadata to communicate with the master to store / get the metadata Closes #404

Resolves #417. This PR: - Defines formats for Executor side Job/Stage/TaskGroup/Task execution metric

Part of #440 * Adds FileArea and FileStore#getFileAreas method. This will be used for zero-copy FilePartition transfer(#353), as descriptors for file regions. * Adds HashRange to designate hash ranges and modifies existing codes to use it.

This PR: Resolves #355, #300 - Makes metadata server support the block region reservation. - Makes GlusterFileStore support block appending write. (During this process, the store communicates with the metadata server for each block append) - Refactors FilePartitions and FileMetadatas. - Revises the partition state managing for I-File write. (This is because that I-File has multiple producer tasks)

This PR: - Makes the stage partitioning part into a separate pass. - Makes stages into an attribute to tag each IR Vertex with. - Makes the Optimizer completely static - Makes necessary changes to the tests. resolves #426

…en compiler and runtime (#429) This PR: Resolves #423 - Creates a PubSubEventHandlerWrapper for a global PubSubEventHandler We can create subscriptions between different events and their corresponding handlers. - Creates a CompilerEventHandler to handle events on the compiler side - Creates a CompilerEvent interface - Creates a DynamicOptimizationEvent - Creates a RuntimeEventHandler to handle event on the runtime side - Creates a RuntimeEvent interface - Creates a UpdatePhysicalPlanEvent - Refactors Optimizer to become static - Refactors the dynamic optimization pass to a new separate class, for better modularity. - Provides an API on the scheduler to be able to update the PhysicalPlan. - Fixes tests to use the updated structure

Resolves #173 A part of #440 - Defines data transfer protocol. - Introduces two types of the frame, namely control frame and data frame.

This PR adds a command line parameter for JVM heap slack Resolves #451

This PR: - Does minor cleanups and fixes that have been lost during review of #429. - Fixes confusion that might occur about the position of each handlers and events.

This PR improves job state logging by * removing duplicated logs * removing messages meaning that job state has been saved to a json file * adding messages about the number of remaining TaskGroups.

Resolves #450 A part of #440 With this PR, the vortex runtime can * Start PartitionTransport server (See PartitionTransport) * Set up channel pipeline (See ChannelInitializer) * Expose API to initiate pull/push-based transfer (See PartitionTransfer)

Resolves #460 A part of #440 ControlMessageToPartitionStreamCodec builds control messages for outgoing requests, and PartitionStreams for incoming requests.

This PR: - Implements the algorithm required for even data partitioning in DataSkewDynamicOptimizationPass. - Provides types for metric data - Provides a minor fix in DataSkewPass about the part where it was tied to BEAM. resolves #390

Resolves #448, since current REEF master uses Netty 4.0.23.Final - Removes dependency: REEF 0.15.0 - Adds dependency: REEF 0.17.0-SNAPSHOT - Adds bundled Maven local repository to include prebuilt artifacts - Removes custom Wake JAR file (see #272) - Makes NativeChannelImplementationSelector default - Fixes a bug that made PartitionTransport unable to retry binding if one of the candidate ports is unavailable

This PR: - Adds the ScheduleGroupNum attribute - Provides a ScheduleGroupPass for annotating each vertices with the attribute - Provides tests to make sure it functions as intended resolves #437

Resolves #465 A part of #440 This PR implements end-to-end stream-based partition transfer API. Netty EventLoopGroup threads, encoder/decoder threads, and user threads (PartitionManagerWorker) are involved.

Resolves #265 . This PR: - Waits for all requested executors to be returned even when `ContainerManager` is terminated to ensure REEF job completes. - Resolves the possible root cause for occasional CI build failures.

Resolves #487 . This PR: - Adds ScheduleGroup to `Stage` and `PhysicalStage` - Checks that all vertices in a single `Stage` have identical ScheduleGroupIndex (Integrity check)

Drops PartitionTransferPeer to use PartitionTransfer Resolves #440 * Drops PartitionTransferPeer in favor of PartitionTransfer, to transfer data between executors * Rename {fetch=>pull}, {send=>push} as discussed

This PR: - Exposes the Block to upper level and make it as a unit of write (& read in #463) - Makes PartitionStore to unaware some conditions such as writable or hashed (instead, make PartitionManagerWorker and above things manage it) - Prepares to support the "Incremental" read (#463). -> Make FilePartition can "commit" blocks after write - Modularizes the functions in OutputWriter to ready to "componentize" them asDataTransferStrategy -> Make OutputWriter can "commit" partitions -> Make OutputWriter can choose the synchronization of write Resolves #475, #444

Resolves #419, #443. This PR: - Collects execution times of Job/Stage/TaskGroup/Task and log them. - Job/Stage metrics: collected in Master at JobStateManager. - TaskGroup/Task metrics: collected in Executor at TaskGroupStateManager and sent to Master. Especially, CONTAINER_FAILURE among FAILURE_RECOVERABLE is now propagated to Master in the form of protobuf control messages.

…p (#496) Resolves #438 . This PR: - Changes `BatchScheduler` to schedule `TaskGroups` according to `ScheduleGroup` - Changes `BatchSchedulerTest` to meet the changes.

This PR handles committed partitions of a TaskGroup that fails while executing

…fferent (#502) Resolves #481 . This PR: - Adds `getAncestors()` to `DAG.java` - Enables `PendingTaskGroupPriorityQueue` to let lower priority stages to be scheduled if the container type differs from the higher priority stages.

sanha · 2018-03-09T08:10:24Z

Unfortunately, I've noticed that there is a critical bug in this change.
When I run our MapReduce example for 512 GB Wikimedia data in Amazon EC2, all executors hanged at the first TaskGroup of reduce stage. (It seems that this hanging does not occur in our ITCase size.)
We must resolve this problem before we merge this PR. Let's investigate this issue thoroughly.

sanha · 2018-03-09T08:31:37Z

I strongly suspect that the sink vertex creates temporary file per every element.
After I killed the hanged job, HDFS failed to access the output directory.

bgchun · 2018-03-09T08:34:15Z

@jeongyooneo How do you test your changes in this PR?

sanha · 2018-03-09T11:00:30Z

Here's a good news!
I confirmed that the bug I've reported is fixed after the last commit.
The experiment works well. Thanks for the hard work!

jeongyooneo · 2018-03-09T15:07:17Z

@bgchun I’ve modified the TaskGroupExecutorTest to element-wise version and checked that all ITCases worked correctly with this PR.
If you have any additional suggestion, please let me know, and I’ll try them right away!

jeongyooneo · 2018-03-16T07:19:32Z

Thanks for the review!
I've addressed most of them, except the following two:

For refactoring metric collection code into a separate class requested by @johnyangk, I think we'd better do that in another issue.
Since there are more types of metric to be added, it'd be great if we clean them up after the types of metric we need and their usages become concrete.
For checking TaskGroup complete event by collecting finished Task ids, there are subtle task completion timing issues that made me keep this as is.

TaskGroup is complete when all of its Tasks are complete, i.e. each Task consumed all of its input.
As input is processed element-wise, speculating the exact point when final task is finished is hard. There can be multiple final tasks that process input independently, whose finishing point are known only to each task. Thus, we still need to collect those final task ids to check TaskGroup completion.

sanha

Thanks for the work @jeongyooneo! I've added some comments but it looks good to me now.
@johnyangk @seojangho please check it if you want.

sanha · 2018-03-18T04:57:55Z

common/src/main/java/edu/snu/nemo/common/ir/vertex/transform/RelayTransform.java

@@ -35,13 +33,13 @@ public RelayTransform() {
  }

  @Override
-  public void prepare(final Context context, final OutputCollector<T> oc) {
-    this.outputCollector = oc;
+  public void prepare(final Context context, final OutputCollector<T> p) {


Let's change p to oc again. (for all other changes also)

sanha · 2018-03-18T05:01:41Z

runtime/executor/src/main/java/edu/snu/nemo/runtime/executor/TaskGroupExecutor.java

-   */
-  private final Map<String, List<InputReader>> physicalTaskIdToInputReaderMap;
-  private final Map<String, List<OutputWriter>> physicalTaskIdToOutputWriterMap;
+  private final List<InputReader> inputReaders;


Please check this comment again.

sanha · 2018-03-18T05:02:37Z

runtime/executor/src/main/java/edu/snu/nemo/runtime/executor/TaskGroupExecutor.java

+  private final Map<String, List<TaskDataHandler>> srcIteratorIdToDataHandlersMap;
+  private final Map<String, List<TaskDataHandler>> iteratorIdToDataHandlersMap;
+  private final LinkedBlockingQueue<Pair<String, DataUtil.IteratorWithNumBytes>> partitionQueue;
+  private List<TaskDataHandler> taskDataHandlers;


Do we have to maintain this list and srcIteratorIdToDataHandlersMap, srcIteratorIdToDataHandlersMap at the same time?

sanha · 2018-03-18T05:09:02Z

runtime/executor/src/main/java/edu/snu/nemo/runtime/executor/TaskGroupExecutor.java

+  private final Map<String, List<TaskDataHandler>> iteratorIdToDataHandlersMap;
+  private final LinkedBlockingQueue<Pair<String, DataUtil.IteratorWithNumBytes>> partitionQueue;
+  private List<TaskDataHandler> taskDataHandlers;
+  private Map<OutputCollectorImpl, List<TaskDataHandler>> outputToChildrenDataHandlersMap;


Can't we see this information in TaskDataHandler?

sanha · 2018-03-18T05:09:46Z

runtime/executor/src/main/java/edu/snu/nemo/runtime/executor/TaskGroupExecutor.java

-    final String physicalTaskId = getPhysicalTaskId(boundedSourceTask.getId());
-    final Map<String, Object> metric = new HashMap<>();
-    metricCollector.beginMeasurement(physicalTaskId, metric);
+  private boolean hasOutputWriter(final Task task) {


Please add comments to all these private methods.

sanha · 2018-03-18T05:16:24Z

runtime/executor/src/main/java/edu/snu/nemo/runtime/executor/TaskGroupExecutor.java

+            final Object element = outputCollector.remove();
+
+            // Pass outputCollectorOwnerTask's output to its children tasks recursively.
+            if (!childrenDataHandlers.isEmpty()) {


do we have to check this?

sanha · 2018-03-18T05:16:50Z

runtime/executor/src/main/java/edu/snu/nemo/runtime/executor/TaskGroupExecutor.java

+    } catch (final BlockWriteException ex2) {
+      taskGroupStateManager.onTaskGroupStateChanged(TaskGroupState.State.FAILED_RECOVERABLE,
+          Optional.empty(), Optional.of(TaskGroupState.RecoverableFailureCause.OUTPUT_WRITE_FAILURE));
+      LOG.info("{} Execution Failed (Recoverable: output write failure)! Exception: {}",


LOG.error?

Please check this comment again.

sanha · 2018-03-18T05:17:03Z

runtime/executor/src/main/java/edu/snu/nemo/runtime/executor/TaskGroupExecutor.java

+    } catch (final Exception e) {
+      taskGroupStateManager.onTaskGroupStateChanged(TaskGroupState.State.FAILED_UNRECOVERABLE,
+          Optional.empty(), Optional.empty());
+      LOG.info("{} Execution Failed! Exception: {}",


Please check this comment again.

sanha · 2018-03-18T05:18:25Z

runtime/executor/src/main/java/edu/snu/nemo/runtime/executor/TaskGroupExecutor.java

-      } catch (final InterruptedException e) {
-        throw new BlockFetchException(e);
+    // Process element-wise depending on the Task type
+    if (task instanceof BoundedSourceTask) {


If we pre-build these stuffs as Callable in TaskDataHandler, we can avoid to doing instanceofs per every elements.

sanha · 2018-03-18T05:19:29Z

...e/executor/src/main/java/edu/snu/nemo/runtime/executor/datatransfer/OutputCollectorImpl.java

   */
-  public List<O> collectOutputList() {
-    return outputList.getAndSet(new ArrayList<>());
+  public O remove() {


Please check this comment again

johnyangk · 2018-03-18T11:35:16Z

Thanks @sanha. I'll make another pass once @jeongyooneo addresses the comments here.

…pache#165) * Add getStateMachine() * TaskGroup getId() * ExecutionStateManager introduced * New scheduling policies * Update states * Change SchedulingException * Remove SampleScheduler * Update ExecutorRepresenter * Update scheduling policy * Add ExecutionStateManager * Add BatchRRScheduler * Changed interface for Scheduler * Add TaskStateManager * change test name * Add google protobuf: needs to be downgraded * Checkstyle/compilation resolved * protobuf files * Revert States * Add more exceptions * Cleanup states (again) * TaskStateManager * ExecutorRepresenter to only contain taskGroup Ids * Change scheduling policy interface * Add protobuf message as a sample * Update execution state manager interfaces * Change an exception * Scheduler modified * Add TODOs * Comments added * More comments * Add current states to toString * Consider multithreading * ExecutionManagerTest * Physical DAG bug fixed * ExecutionStateManager: add print * RRSchedulerTest * TaskGroupStateManager fixed * RRScheduler minor bug fixed * Scheduler * SchedulerTest * Comments addressed 1 * Protobuf compilation changed (w/ proto3) * Ommitted tests * Scheduler -> BatchScheduler * Comments addressed #2 * Comments addressed #3 * Add ommitted test * Comments addressed #4 * Reduce log level * Remove unnecessary

jeongyooneo · 2018-05-09T08:13:47Z

@sanha I've addressed the comments except the following two. These will be addressed via another issue of refactoring TaskDataHandler-based data transfer:

Removing local variables dealing with iterators in TaskGroupExecutor
Pre-build runTask per-task type operation as Callable in TaskDataHandler

sanha

I left some minor comments. (I just skimmed the TaskGroupExecutor class because we already talked about it many times and you have a plan to refactor it.) Thanks!
@johnyangk @seojangho please check this PR and approve if you are happy with it.

sanha · 2018-05-09T13:26:22Z

...e/executor/src/main/java/edu/snu/nemo/runtime/executor/datatransfer/OutputCollectorImpl.java

+   * Set this OutputCollector as having side input for the given child task.
+   *
+   * @param physicalTaskId the id of child task whose side input will be put into this OutputCollector.
+   */
  public void setAsSideInputFor(final String physicalTaskId) {
    sideInputReceivers.add(physicalTaskId);
  }

  public boolean hasSideInputFor(final String physicalTaskId) {


Comment please :)

sanha · 2018-05-09T13:29:54Z

...end/spark/src/main/java/edu/snu/nemo/compiler/frontend/spark/transform/CollectTransform.java


 /**
 * Collect transform.
 * @param <T> type of data to collect.
 */
 public final class CollectTransform<T> implements Transform<T, T> {
  private String filename;
+  private FileOutputStream fos;


Why do we need to have these fos and oos as variables? It can be created only in close().

sanha · 2018-05-09T13:30:31Z

...end/spark/src/main/java/edu/snu/nemo/compiler/frontend/spark/transform/CollectTransform.java

  }

  @Override
  public void close() {
+    try {


Please use try-with-resource clause for these Streams.

sanha · 2018-05-09T13:33:11Z

compiler/optimizer/src/main/java/edu/snu/nemo/compiler/optimizer/examples/EmptyComponents.java

@@ -60,7 +59,7 @@ public void prepare(final Context context, final OutputCollector<O> outputCollec
    }

    @Override
-    public void onData(final Iterator<I> elements, final String srcVertexId) {
+    public void onData(final Object element) {


I element?

sanha · 2018-05-09T13:34:23Z

...ples/beam/src/test/java/edu/snu/nemo/examples/beam/policy/DataSkewPolicyParallelismFive.java

@@ -25,10 +25,10 @@
 /**
 * A data-skew policy with fixed parallelism 5 for tests.
 */
-public final class DataSkewPolicyParallelsimFive implements Policy {
+public final class DataSkewPolicyParallelismFive implements Policy{


Please revert the space between Policy and bracket.

sanha · 2018-05-09T13:36:14Z

runtime/executor/src/main/java/edu/snu/nemo/runtime/executor/TaskGroupExecutor.java

+    } catch (final Exception e) {
+      taskGroupStateManager.onTaskGroupStateChanged(TaskGroupState.State.FAILED_UNRECOVERABLE,
+          Optional.empty(), Optional.empty());
+      LOG.info("{} Execution Failed! Exception: {}",


Please check this comment again.

sanha · 2018-05-09T13:37:21Z

...e/executor/src/main/java/edu/snu/nemo/runtime/executor/datatransfer/OutputCollectorImpl.java

+    sideInputRuntimeEdge = edge;
+  }
+
+  public RuntimeEdge getSideInputRuntimeEdge() {


Please add a comment :)

sanha · 2018-05-09T13:37:24Z

...e/executor/src/main/java/edu/snu/nemo/runtime/executor/datatransfer/OutputCollectorImpl.java

+  public void setAsSideInputFor(final String physicalTaskId) {
+    sideInputReceivers.add(physicalTaskId);
+  }
+


Please add a comment :)

sanha · 2018-05-09T13:37:47Z

runtime/executor/src/main/java/edu/snu/nemo/runtime/executor/datatransfer/OutputWriter.java

   *
-   * @param dataToWrite An iterable for the elements to be written.
+   * * @param element


Please remove the duplicated *

sanha · 2018-05-09T13:38:24Z

runtime/executor/src/main/java/edu/snu/nemo/runtime/executor/datatransfer/TaskDataHandler.java

+public final class TaskDataHandler {
+  private static final Logger LOG = LoggerFactory.getLogger(TaskDataHandler.class.getName());
+
+  public TaskDataHandler(final Task task) {


Please add comments for the constructor and getters.

johnyangk

Cancel 'Request changes'.

jeongyooneo · 2018-05-10T05:28:11Z

@sanha Just finished addressing comments!

sanha

LGTM. I'll also look forward to your remaining refactoring pr. Thanks for the work!

wynot12 and others added 30 commits August 18, 2017 18:02

commit (#425)

9e6356f

Closes #268 #268 has reported that importing some Beam's hdfs-related maven artifacts causes integration failures. But I checked that it causes no problem and the tests run well. So now we can use it to deal with IO to/from Hdfs.

[Vortex-418] Collect Executor Metric in Master (#422)

479a9e2

* [Vortex-418] Collect Executor Metric in Master * Flush remaining metrics while closing the metric sender * rm index increment

[Vortex-404] Introduce metadata server model (#426)

c453006

This PR addressed #404 by - implements MetadataManager that manages the metadata in the master side - makes RemoteFileMetadata to communicate with the master to store / get the metadata Closes #404

[Vortex-417] Executor metric format (#435)

109f93d

Resolves #417. This PR: - Defines formats for Executor side Job/Stage/TaskGroup/Task execution metric

[Minor] Adds FileArea and HashRange (#441)

44910cf

Part of #440 * Adds FileArea and FileStore#getFileAreas method. This will be used for zero-copy FilePartition transfer(#353), as descriptors for file regions. * Adds HashRange to designate hash ranges and modifies existing codes to use it.

[Vortex-426] Stage Partitioning Pass (#439)

318ac02

This PR: - Makes the stage partitioning part into a separate pass. - Makes stages into an attribute to tag each IR Vertex with. - Makes the Optimizer completely static - Makes necessary changes to the tests. resolves #426

[Vortex-173] Implement Data Transfer Protocol (#442)

5b56f32

Resolves #173 A part of #440 - Defines data transfer protocol. - Introduces two types of the frame, namely control frame and data frame.

Add a command line parameter: JVM heap slack (#453)

f168d36

This PR adds a command line parameter for JVM heap slack Resolves #451

[MINOR] cleanups following #429 (#454)

84d95ba

This PR: - Does minor cleanups and fixes that have been lost during review of #429. - Fixes confusion that might occur about the position of each handlers and events.

[Minor] Improve job state logging (#457)

64f4a00

This PR improves job state logging by * removing duplicated logs * removing messages meaning that job state has been saved to a json file * adding messages about the number of remaining TaskGroups.

[Vortex-460] Setting up transport context (#461)

be09e36

Resolves #460 A part of #440 ControlMessageToPartitionStreamCodec builds control messages for outgoing requests, and PartitionStreams for incoming requests.

remove (#468)

3c853c3

[Vortex-437] Introduce schedule group and pass (#459)

e5b1820

This PR: - Adds the ScheduleGroupNum attribute - Provides a ScheduleGroupPass for annotating each vertices with the attribute - Provides tests to make sure it functions as intended resolves #437

[Vortex-465] End-to-end stream-based API (#466)

8f5f5c1

Resolves #465 A part of #440 This PR implements end-to-end stream-based partition transfer API. Netty EventLoopGroup threads, encoder/decoder threads, and user threads (PartitionManagerWorker) are involved.

clean up stream apis with code convention (#474)

7d563d1

[Vortex-265] Close All Requested Executors (#486)

64338cf

Resolves #265 . This PR: - Waits for all requested executors to be returned even when `ContainerManager` is terminated to ensure REEF job completes. - Resolves the possible root cause for occasional CI build failures.

[Vortex-487] ScheduleGroup in PhysicalPlan (#488)

e371a74

Resolves #487 . This PR: - Adds ScheduleGroup to `Stage` and `PhysicalStage` - Checks that all vertices in a single `Stage` have identical ScheduleGroupIndex (Integrity check)

[Vortex-440] Refactor data plane (#490)

afdbaa6

Drops PartitionTransferPeer to use PartitionTransfer Resolves #440 * Drops PartitionTransferPeer in favor of PartitionTransfer, to transfer data between executors * Rename {fetch=>pull}, {send=>push} as discussed

[Minor] Fix container failrue message (#499)

faf57c2

[Vortex-438] Fix BatchScheduler to Schedule According to ScheduleGrou…

45368e7

…p (#496) Resolves #438 . This PR: - Changes `BatchScheduler` to schedule `TaskGroups` according to `ScheduleGroup` - Changes `BatchSchedulerTest` to meet the changes.

Fix partition state control in task group failed case (#500)

979542e

This PR handles committed partitions of a TaskGroup that fails while executing

remove logs

7c84c0e

element-wise DoTransform and ALS

38c4af9

jeongyooneo added 2 commits March 16, 2018 15:40

address comments

aa0b7ce

Merge branch 'master' into 717-TGE

75fdde5

added comments

89049d9

sanha previously approved these changes Mar 18, 2018

View reviewed changes

jeongyooneo dismissed sanha’s stale review via d667589 May 8, 2018 13:29

added comments

13510f4

jeongyooneo force-pushed the 717-TGE branch from d667589 to 13510f4 Compare May 9, 2018 07:09

merge master

c4d29ec

sanha reviewed May 9, 2018

View reviewed changes

johnyangk reviewed May 10, 2018

View reviewed changes

johnyangk previously approved these changes May 10, 2018

View reviewed changes

address comments

5e7f808

jeongyooneo dismissed johnyangk’s stale review via 5e7f808 May 10, 2018 05:27

seojangho approved these changes May 10, 2018

View reviewed changes

sanha approved these changes May 10, 2018

View reviewed changes

sanha merged commit cf041f6 into master May 10, 2018

sanha deleted the 717-TGE branch May 10, 2018 06:03

jeongyooneo mentioned this pull request May 20, 2018

[NEMO-76] Rename TaskGroup to Task #20

Merged

[NEMO-7] Intra-TaskGroup pipelining #2

[NEMO-7] Intra-TaskGroup pipelining #2

Conversation

jeongyooneo commented Mar 5, 2018 • edited

sanha commented Mar 9, 2018

sanha commented Mar 9, 2018

bgchun commented Mar 9, 2018

sanha commented Mar 9, 2018

jeongyooneo commented Mar 9, 2018 • edited

jeongyooneo commented Mar 16, 2018

sanha left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnyangk commented Mar 18, 2018

jeongyooneo commented May 9, 2018 • edited

sanha left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnyangk left a comment

Choose a reason for hiding this comment

jeongyooneo commented May 10, 2018

sanha left a comment

Choose a reason for hiding this comment

jeongyooneo commented Mar 5, 2018 •

edited

jeongyooneo commented Mar 9, 2018 •

edited

jeongyooneo commented May 9, 2018 •

edited