[SPARK-20652][sql] Store SQL UI data in the new app status store. #19681

vanzin · 2017-11-07T00:31:13Z

This change replaces the SQLListener with a new implementation that
saves the data to the same store used by the SparkContext's status
store. For that, the types used by the old SQLListener had to be
updated a bit so that they're more serialization-friendly.

The interface for getting data from the store was abstracted into
a new class, SQLAppStatusStore (following the convention used in
core).

Another change is the way that the SQL UI hooks up into the core
UI or the SHS. The old "SparkHistoryListenerFactory" was replaced
with a new "AppStatePlugin" that more explicitly differentiates
between the two use cases: processing events, and showing the UI.
Both live apps and the SHS use this new API (previously, it was
restricted to the SHS).

Note on the above: this causes a slight change of behavior for
live apps; the SQL tab will only show up after the first execution
is started.

The metrics gathering code was re-worked a bit so that the types
used are less memory hungry and more serialization-friendly. This
reduces memory usage when using in-memory stores, and reduces load
times when using disk stores.

Tested with existing and added unit tests. Note one unit test was
disabled because it depends on SPARK-20653, which isn't in yet.

This change replaces the SQLListener with a new implementation that saves the data to the same store used by the SparkContext's status store. For that, the types used by the old SQLListener had to be updated a bit so that they're more serialization-friendly. The interface for getting data from the store was abstracted into a new class, SQLAppStatusStore (following the convention used in core). Another change is the way that the SQL UI hooks up into the core UI or the SHS. The old "SparkHistoryListenerFactory" was replaced with a new "AppStatePlugin" that more explicitly differentiates between the two use cases: processing events, and showing the UI. Both live apps and the SHS use this new API (previously, it was restricted to the SHS). Note on the above: this causes a slight change of behavior for live apps; the SQL tab will only show up after the first execution is started. The metrics gathering code was re-worked a bit so that the types used are less memory hungry and more serialization-friendly. This reduces memory usage when using in-memory stores, and reduces load times when using disk stores. Tested with existing and added unit tests. Note one unit test was disabled because it depends on SPARK-20653, which isn't in yet.

vanzin · 2017-11-07T00:32:43Z

For context:

Project link: https://issues.apache.org/jira/browse/SPARK-18085
Upcoming PRs that build on this code: https://github.com/vanzin/spark/pulls

Note I took this PR out of its original order in my repo to speed up reviews. That means it is not exactly the same code as in there - I'll need to clean up the FsHistoryProvider code in a separate change.

SparkQA · 2017-11-07T03:46:23Z

Test build #83521 has finished for PR 19681 at commit ccd5adc.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
class SQLTab(val sqlStore: SQLAppStatusStore, sparkUI: SparkUI)

SparkQA · 2017-11-07T04:28:03Z

Test build #83525 has finished for PR 19681 at commit c4af460.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

squito

looks reasonable but I need to spend more time understanding the old SQL UI first ...

squito · 2017-11-07T17:49:25Z

sql/core/src/main/scala/org/apache/spark/sql/execution/ui/SQLAppStatusListener.scala

+    ui: Option[SparkUI] = None)
+  extends SparkListener with Logging {
+
+  // How often to flush intermediate statge of a live execution to the store. When replaying logs,


typo: statge

squito · 2017-11-07T18:14:48Z

sql/core/src/main/scala/org/apache/spark/sql/execution/ui/SQLAppStatusListener.scala

+import org.apache.spark.ui.SparkUI
+import org.apache.spark.util.kvstore.KVStore
+
+private[sql] class SQLAppStatusListener(


I don't see calls to update in onTaskEnd or onExecutorMetricsUpdate. Does that mean the live UI wont' update till a stage is finished? But after looking at the tests, I guess I'm wrong, it does update ... where is the update I'm missing?

The live UI is updated from the info in LiveStageMetrics, which is not written to the store. That's kept in memory while executions are running, and aggregated into the final metrics view when the execution finishes (see aggregateMetrics).

SQLAppStatusStore.executionMetrics has logic to call the listener directly when the final metrics are not yet computed.

SQLAppStatusStore.executionMetrics has logic to call the listener directly when the final metrics are not yet computed.

ahh, that is the part I hadn't noticed. thanks

squito · 2017-11-07T18:19:13Z

sql/core/src/test/scala/org/apache/spark/sql/execution/ui/SQLListenerSuite.scala

 import org.apache.spark.util.{AccumulatorMetadata, JsonProtocol, LongAccumulator}
-
+import org.apache.spark.util.kvstore.InMemoryStore

 class SQLListenerSuite extends SparkFunSuite with SharedSQLContext with JsonTestUtils {


rename test to SQLAppStatusListenerSuite

Actually this suite has a mix of tests for code in the (current) listener source file and for stuff that's not related to those, which would belong in SQLAppStatusListenerSuite. My original changes broke this into two different suites, but I chose to postpone that to reduce the size of the diff for now (and also to make the diff a little easier to read).

SparkQA · 2017-11-07T21:19:08Z

Test build #83563 has finished for PR 19681 at commit 46695ed.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

vanzin · 2017-11-07T21:37:38Z

retest this please

SparkQA · 2017-11-07T21:42:43Z

Test build #83567 has started for PR 19681 at commit 46695ed.

squito · 2017-11-07T22:53:03Z

sql/core/src/main/scala/org/apache/spark/sql/execution/ui/SQLAppStatusListener.scala

+    val now = System.nanoTime()
+    if (exec.endEvents >= exec.jobs.size + 1) {
+      liveExecutions.remove(exec.executionId)
+      exec.write(kvstore, now)


if a job was killed mid execution, so the event log didn't contain the end events, you wouldn't get to see any info for that execution, would you? don't you need some final flush?

When you kill a job, isn't an end event generated?

This is trying to mimic the behavior from the old listener:

jobEnd.jobResult match { case JobSucceeded => executionUIData.jobs(jobId) = JobExecutionStatus.SUCCEEDED case JobFailed(_) => executionUIData.jobs(jobId) = JobExecutionStatus.FAILED } if (executionUIData.completionTime.nonEmpty && !executionUIData.hasRunningJobs) { // We are the last job of this execution, so mark the execution as finished. Note that // `onExecutionEnd` also does this, but currently that can be called before `onJobEnd` // since these are called on different threads. markExecutionFinished(executionId) }

So in that case, if there's no end event for the job, the execution will not be marked as finished either.

sorry, I meant if the application is killed, so the event log just ends abruptly. I think the old history server code would still show you the updated metrics for all the tasks that had completed. But seems like after this change, the history server won't show anything for any jobs which hadn't completed.

That's true; I've actually fixed that in the M6 code:
https://github.com/vanzin/spark/pull/51/files#diff-a74d84702d8d47d5269e96740a55a3caR56

It's not very easy to fix here without writing throw-away code to propagate some "close()" call to this listener.

squito · 2017-11-07T22:57:32Z

sql/core/src/main/scala/org/apache/spark/sql/execution/ui/SQLAppStatusListener.scala

+
+  private def isSQLStage(stageId: Int): Boolean = {
+    liveExecutions.values.exists { exec =>
+      exec.stages.contains(stageId)


any reason not to make exec.stages a Set? I guess it would normally be small so it doesn't matter, but seems like it couldn't hurt to protect against some strange SQL query with some huge number of stages.

SparkQA · 2017-11-07T23:42:45Z

Test build #83570 has started for PR 19681 at commit 197dd8f.

squito · 2017-11-08T05:24:40Z

sql/core/src/main/scala/org/apache/spark/sql/execution/ui/SQLAppStatusListener.scala

@@ -40,7 +40,7 @@ private[sql] class SQLAppStatusListener(
    ui: Option[SparkUI] = None)
  extends SparkListener with Logging {

-  // How often to flush intermediate statge of a live execution to the store. When replaying logs,
+  // How often to flush intermediate stage of a live execution to the store. When replaying logs,


err, was this supposed to be "state"?

SparkQA · 2017-11-08T08:05:01Z

Test build #83585 has finished for PR 19681 at commit ecf293b.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

vanzin · 2017-11-08T18:10:34Z

retest this please

squito

I think I have a reasonable handle on the SQL UI stuff now.

Are there any tests for the SQL UI in the history server? I don't see anything ...

squito · 2017-11-08T19:10:11Z

sql/core/src/main/scala/org/apache/spark/sql/execution/ui/SQLAppStatusListener.scala

+    val stageId: Int,
+    var attemptId: Int,
+    val accumulatorIds: Array[Long],
+    val taskMetrics: ConcurrentHashMap[Long, LiveTaskMetrics])


can you include a comment explaining the threading concerns in this class? at first I didn't think the CHM or the synchronized were necessary since the listener methods are just called from one thread, but then I realized the UI will also call executionMetrics(executionId) (I think that is the only reason?). Also I think there are more synchronized than necessary -- both sites aggregateMetrics are called have already acquired the lock, so that shouldn't need it again. though it doesn't hurt, it can be confusing if its not clear where there lock is supposed to be aquired.

I changed the implementation a bit since it wasn't completely correct. Added some comments in a few places.

squito · 2017-11-08T19:10:26Z

sql/core/src/main/scala/org/apache/spark/sql/execution/ui/SQLAppStatusListener.scala

+import org.apache.spark.scheduler._
+import org.apache.spark.sql.execution.SQLExecution
+import org.apache.spark.sql.execution.metric._
+import org.apache.spark.sql.internal.StaticSQLConf._


unused import

squito · 2017-11-08T19:10:59Z

sql/core/src/main/scala/org/apache/spark/sql/execution/ui/SQLAppStatusStore.scala

+import com.fasterxml.jackson.databind.annotation.JsonDeserialize
+
+import org.apache.spark.{JobExecutionStatus, SparkConf}
+import org.apache.spark.scheduler.{SparkListener, SparkListenerEvent}


SparkListenerEvent is unused

squito · 2017-11-08T19:51:57Z

sql/core/src/main/scala/org/apache/spark/sql/execution/ui/SQLListener.scala

-          logWarning(s"A task should not have a higher stageAttemptID ($stageAttemptID) then " +
-            s"what we have seen (${stageMetrics.stageAttemptId})")
-        } else {
-          // TODO We don't know the attemptId. Currently, what we can do is overriding the


this comment was hard to make sense of (you shouldn't ever have two tasks with the same taskId, even with speculation), but I think there is something here which may still be worth mentioning. You aggregate metrics across all attempts for a given task (aka "index"), even speculative ones (before and after your change) -- I'd mention that in a comment.

(The index is available in onTaskStart / End if we wanted to de-duplicate.)

squito · 2017-11-08T19:59:11Z

sql/core/src/test/scala/org/apache/spark/sql/execution/metric/SQLMetricsSuite.scala

@@ -420,21 +428,19 @@ class SQLMetricsSuite extends SparkFunSuite with SQLMetricsTestUtils with Shared
    withTempPath { file =>
      // person creates a temporary view. get the DF before listing previous execution IDs
      val data = person.select('name)
-      sparkContext.listenerBus.waitUntilEmpty(10000)


don't you still need this waitUntilEmpty?

person.select() doesn't generate any events.

vanzin · 2017-11-08T21:27:28Z

Are there any tests for the SQL UI in the history server?

No, because those tests are in core. I want to add some SHS integration tests into a separate module after this works goes in, covering core, SQL and the disk store, so that we can detect incompatibilities when they happen. But haven't really written anything yet.

SparkQA · 2017-11-08T21:32:35Z

Test build #83603 has finished for PR 19681 at commit ecf293b.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2017-11-09T00:55:00Z

Test build #83609 has finished for PR 19681 at commit a42f418.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2017-11-09T05:58:58Z

Test build #83620 has finished for PR 19681 at commit bb7388b.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

squito · 2017-11-09T21:12:22Z

Are there any tests for the SQL UI in the history server?

No, because those tests are in core. I want to add some SHS integration tests into a separate module after this works goes in, covering core, SQL and the disk store, so that we can detect incompatibilities when they happen. But haven't really written anything yet.

OK -- is there a jira for adding those tests?

vanzin · 2017-11-09T21:14:16Z

Not that I remember.

squito · 2017-11-09T21:27:40Z

sql/core/src/main/scala/org/apache/spark/sql/execution/ui/SQLAppStatusStore.scala

+    val exec = store.read(classOf[SQLExecutionUIData], executionId)
+    Option(exec.metricValues)
+      .orElse(listener.map(_.executionMetrics(executionId)))
+      .getOrElse(Map())


is there a race here when the execution ends?

T1 (UI thread, calling this method): execution hasn't ended, so exec.metricValues is null

T2 (listener): execution ends, drops execution from liveExecutions

T1: _.executionMetrics(executionId) throws an exception because its dropped from the liveExecutions

squito · 2017-11-09T21:42:10Z

sql/core/src/main/scala/org/apache/spark/sql/execution/ui/SQLAppStatusListener.scala

+        }
+      }
+
+      // TODO: storing metrics by task ID can lead to innacurate metrics when speculation is on.


I think its more general than this. I'd say

Since we store metrics by taskID, in a way we'll double-count the stage metrics when there are multiple tasks for a given index -- in particular, if there is speculation, or if there are multiple attempts for a task.

SparkQA · 2017-11-10T02:47:59Z

Test build #83658 has finished for PR 19681 at commit 1a31665.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

squito · 2017-11-13T05:22:50Z

lgtm

need to fix the merge conflict though

…ryError ## What changes were proposed in this pull request? This PR addresses the issue [SPARK-22471](https://issues.apache.org/jira/browse/SPARK-22471). The modified version of `SQLListener` respects the setting `spark.ui.retainedStages` and keeps the number of the tracked stages within the specified limit. The hash map `_stageIdToStageMetrics` does not outgrow the limit, hence overall memory consumption does not grow with time anymore. A 2.2-compatible fix. Maybe incompatible with 2.3 due to #19681. ## How was this patch tested? A new unit test covers this fix - see `SQLListenerMemorySuite.scala`. Author: Arseniy Tashoyan <tashoyan@gmail.com> Closes #19711 from tashoyan/SPARK-22471-branch-2.2.

SparkQA · 2017-11-13T22:13:54Z

Test build #83802 has finished for PR 19681 at commit 56761af.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

squito · 2017-11-14T16:37:56Z

@vanzin merge conflict since I merged the Job & stage page change

SparkQA · 2017-11-14T21:09:12Z

Test build #83855 has finished for PR 19681 at commit c068a5b.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

squito · 2017-11-14T21:29:05Z

merged to master

cloud-fan · 2017-12-14T16:11:11Z

sql/core/src/main/scala/org/apache/spark/sql/execution/ui/SQLAppStatusStore.scala

+    }
+  }
+
+  override def setupUI(ui: SparkUI): Unit = {


Do we have a clear rule about when setupListeners is called and when setupUI is called?

Here we register SQLAppStatusListener in both setupListeners and setupUI, will we register it twice?

The calls are made in specific cases (setupListeners when setting up a listener bus, setupUI when setting up the UI, always). But this implementation has to be a little weird because we don't want the SQL UI if SQL hasn't been initialized, if we're to maintain the old behavior.

I don't think the listener is installed twice here - here it's only installed for non-live applications (= SHS) and below it's only installed if there's a SparkContext (= live application).

If we're ok to modify the existing behavior and always have the SQL tab, this can be simplified a lot.

cloud-fan · 2017-12-14T17:51:12Z

core/src/main/scala/org/apache/spark/SparkContext.scala

-    _ui.foreach(_.bind())
+    _ui.foreach { ui =>
+      // Load any plugins that might want to modify the UI.
+      AppStatusPlugin.loadPlugins().foreach(_.setupUI(ui))


@vanzin the live UI doesn't need a 2-step process to set up the UI, while history server needs. That's why I think they should not share one plugin interface.

Let's continue the discussion on the other PR.

cloud-fan · 2017-12-15T06:08:59Z

sql/core/src/test/scala/org/apache/spark/sql/execution/ui/SQLListenerSuite.scala

-    def waitTillExecutionFinished(): Unit = {
-      while (listener.getCompletedExecutions.isEmpty) {
-        Thread.sleep(100)
+    while (statusStore.executionsList().size < oldCount) {


This doesn't mean execution ends now, but execution starts.

True, but the lines below:

// Wait for listener to finish computing the metrics for the execution. while (statusStore.executionsList().last.metricValues == null) { Thread.sleep(100) }

Basically wait for that new execution to end (which is when the coalesced metricValues field is populated).

cloud-fan · 2017-12-15T06:10:16Z

sql/core/src/test/scala/org/apache/spark/sql/execution/ui/SQLListenerSuite.scala

    val physicalPlan = MyPlan(sqlContext.sparkContext, expectedAccumValue)
-    sqlContext.sparkContext.addSparkListener(listener)


Does it really work? The listener is attached to a ReplayBus, not the real spark event bus.

Yes, it works. There are two kinds of tests in this suite now:

"test(blah)" like this one which uses the active spark session's listener

"sqlStoreTest" which manually drive a replay bus and verify expected changes in the store.

This particular test (and the other one you commented on) are the first kind. Tests of the other kind do not run actual jobs, they just inject events into the replay bus manually.

cloud-fan · 2017-12-15T06:10:44Z

sql/core/src/test/scala/org/apache/spark/sql/execution/ui/SQLListenerSuite.scala

  }

  test("SPARK-11126: no memory leak when running non SQL jobs") {
-    val previousStageNumber = spark.sharedState.listener.stageIdToStageMetrics.size
+    val previousStageNumber = statusStore.executionsList().size
    spark.sparkContext.parallelize(1 to 10).foreach(i => ())
    spark.sparkContext.listenerBus.waitUntilEmpty(10000)


ditto, the listener is not attached to spark event bus.

cloud-fan · 2017-12-15T06:15:00Z

sql/core/src/test/scala/org/apache/spark/sql/execution/ui/SQLListenerSuite.scala


 class SQLListenerSuite extends SparkFunSuite with SharedSQLContext with JsonTestUtils {
  import testImplicits._
-  import org.apache.spark.AccumulatorSuite.makeInfo
+
+  override protected def sparkConf = super.sparkConf.set(LIVE_ENTITY_UPDATE_PERIOD, 0L)


the spark context is shared for all test suites, we should only set this conf to 0 in this suite.

Is that true? The suite extends SharedSQLContext (which extends SharedSparkSession) and SQLTestUtils, all of which are traits, not objects. (Unlike TestHive which does force sessions to be used across suites for hive tests.)

There are also other suites that modify the conf (such as HDFSMetadataLogSuite).

ah you are right, it's only shared in hive tests

carsonwang · 2017-12-21T09:06:49Z

sql/core/src/main/scala/org/apache/spark/sql/execution/ui/SQLAppStatusListener.scala

+    }
+
+    exec.jobs = exec.jobs + (jobId -> JobExecutionStatus.RUNNING)
+    exec.stages = event.stageIds.toSet


@vanzin , shall we add the stageIds to the existing stageIds? Otherwise we will lose the stageIds in previous jobs?

oh good catch. I can submit a fix for this

## What changes were proposed in this pull request? In #19681 we introduced a new interface called `AppStatusPlugin`, to register listeners and set up the UI for both live and history UI. However I think it's an overkill for live UI. For example, we should not register `SQLListener` if users are not using SQL functions. Previously we register the `SQLListener` and set up SQL tab when `SparkSession` is firstly created, which indicates users are going to use SQL functions. But in #19681 , we register the SQL functions during `SparkContext` creation. The same thing should apply to streaming too. I think we should keep the previous behavior, and only use this new interface for history server. To reflect this change, I also rename the new interface to `SparkHistoryUIPlugin` This PR also refines the tests for sql listener. ## How was this patch tested? existing tests Author: Wenchen Fan <wenchen@databricks.com> Closes #19981 from cloud-fan/listener.

…ryError ## What changes were proposed in this pull request? This PR addresses the issue [SPARK-22471](https://issues.apache.org/jira/browse/SPARK-22471). The modified version of `SQLListener` respects the setting `spark.ui.retainedStages` and keeps the number of the tracked stages within the specified limit. The hash map `_stageIdToStageMetrics` does not outgrow the limit, hence overall memory consumption does not grow with time anymore. A 2.2-compatible fix. Maybe incompatible with 2.3 due to apache#19681. ## How was this patch tested? A new unit test covers this fix - see `SQLListenerMemorySuite.scala`. Author: Arseniy Tashoyan <tashoyan@gmail.com> Closes apache#19711 from tashoyan/SPARK-22471-branch-2.2.

Don't store driver accum updates in the status store.

c4af460

squito reviewed Nov 7, 2017

View reviewed changes

Typo.

46695ed

squito reviewed Nov 7, 2017

View reviewed changes

Use set to keep stage list.

197dd8f

squito reviewed Nov 8, 2017

View reviewed changes

Typo.

ecf293b

squito reviewed Nov 8, 2017

View reviewed changes

Simplify and fix synchronization for live executions.

a42f418

Remove stale comment.

bb7388b

vanzin mentioned this pull request Nov 9, 2017

[SPARK-22471][SQL] SQLListener consumes much memory causing OutOfMemoryError #19700

Closed

tashoyan mentioned this pull request Nov 9, 2017

[SPARK-22471][SQL] SQLListener consumes much memory causing OutOfMemoryError #19711

Closed

squito reviewed Nov 9, 2017

View reviewed changes

Fix a race.

1a31665

Merge branch 'master' into SPARK-20652

56761af

Merge branch 'master' into SPARK-20652

c068a5b

asfgit closed this in 0ffa7c4 Nov 14, 2017

vanzin deleted the SPARK-20652 branch November 14, 2017 21:37

cloud-fan reviewed Dec 14, 2017

View reviewed changes

cloud-fan mentioned this pull request Dec 14, 2017

[SPARK-22786][SQL] only use AppStatusPlugin in history server #19981

Closed

cloud-fan reviewed Dec 14, 2017

View reviewed changes

cloud-fan reviewed Dec 15, 2017

View reviewed changes

carsonwang reviewed Dec 21, 2017

View reviewed changes

		val physicalPlan = MyPlan(sqlContext.sparkContext, expectedAccumValue)
		sqlContext.sparkContext.addSparkListener(listener)

[SPARK-20652][sql] Store SQL UI data in the new app status store. #19681

[SPARK-20652][sql] Store SQL UI data in the new app status store. #19681

Conversation

vanzin commented Nov 7, 2017

vanzin commented Nov 7, 2017

SparkQA commented Nov 7, 2017

SparkQA commented Nov 7, 2017

squito 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

vanzin Nov 7, 2017 • edited Loading

Choose a reason for hiding this comment

SparkQA commented Nov 7, 2017

vanzin commented Nov 7, 2017

SparkQA commented Nov 7, 2017

Choose a reason for hiding this comment

vanzin Nov 7, 2017 • edited Loading

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

SparkQA commented Nov 7, 2017

Choose a reason for hiding this comment

SparkQA commented Nov 8, 2017

vanzin commented Nov 8, 2017

squito 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

vanzin commented Nov 8, 2017

SparkQA commented Nov 8, 2017

SparkQA commented Nov 9, 2017

SparkQA commented Nov 9, 2017

squito commented Nov 9, 2017

vanzin commented Nov 9, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparkQA commented Nov 10, 2017

squito commented Nov 13, 2017

SparkQA commented Nov 13, 2017

squito commented Nov 14, 2017

SparkQA commented Nov 14, 2017

squito commented Nov 14, 2017

Choose a reason for hiding this comment

vanzin Dec 14, 2017 • edited Loading

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

vanzin Nov 7, 2017 •

edited

Loading

vanzin Nov 7, 2017 •

edited

Loading

vanzin Dec 14, 2017 •

edited

Loading