[SPARK-22673][SQL] InMemoryRelation should utilize existing stats whenever possible #19864

CodingCat · 2017-12-01T22:59:53Z

What changes were proposed in this pull request?

The current implementation of InMemoryRelation always uses the most expensive execution plan when writing cache
With CBO enabled, we can actually have a more exact estimation of the underlying table size...

How was this patch tested?

existing test

CodingCat · 2017-12-01T23:01:21Z

@cloud-fan @viirya @gatorsmile @felixcheung @hvanhovell @HyukjinKwon @dongjoon-hyun @liancheng

SparkQA · 2017-12-01T23:07:48Z

Test build #84382 has finished for PR 19864 at commit 32f7c74.

This patch fails to build.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2017-12-02T04:00:47Z

Test build #84384 has finished for PR 19864 at commit 2082c0e.

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

viirya · 2017-12-02T10:12:32Z

sql/core/src/main/scala/org/apache/spark/sql/execution/columnar/InMemoryRelation.scala

-      // available, return the default statistics.
-      Statistics(sizeInBytes = child.sqlContext.conf.defaultSizeInBytes)
+      children.filter(_.isInstanceOf[LogicalRelation]) match {
+        case Seq(c @ LogicalRelation(_, _, _, _), _) if c.conf.cboEnabled =>


InMemoryRelation is a logical.LeafNode. I think it has no children?

thanks, @viirya , yes, you're right! I misread the generated plan, working on it

CodingCat · 2017-12-02T17:43:14Z

sql/core/src/main/scala/org/apache/spark/sql/execution/CacheManager.scala

+        tableName)
+      if (planToCache.conf.cboEnabled && planToCache.stats.rowCount.isDefined) {
+        inMemoryRelation.setStatsFromCachedPlan(planToCache)
+      }


I have to make InMemoryRelation stateful to avoid breaking APIs.....

I think a more ideal change is to put the original plan stats into the constructor of InMemoryRelation, instead of making it mutable.

Looks like I have no way to access InMemoryRelation from outside of spark package, though it is not a package private class...how is that achieved?

If this is the case, I can modify the constructor

Thanks @cloud-fan

InMemoryRelation is not part of the public API and should be treated as unstable/internal. You can use it at your own risk. Changing the constructor is fine.

SparkQA · 2017-12-02T19:15:42Z

Test build #84390 has finished for PR 19864 at commit bc70817.

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

SparkQA · 2017-12-02T22:15:35Z

Test build #84392 has finished for PR 19864 at commit 4650307.

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

viirya · 2017-12-03T03:06:42Z

Is this initial statistics important? After the columnar RDD is materialized, we will get accurate statistics then. Don't we?

…

On Dec 3, 2017 1:43 AM, "Nan Zhu" ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In sql/core/src/main/scala/org/apache/spark/sql/execution/ CacheManager.scala <#19864 (comment)>: > - planToCache, - InMemoryRelation( - sparkSession.sessionState.conf.useCompression, - sparkSession.sessionState.conf.columnBatchSize, - storageLevel, - sparkSession.sessionState.executePlan(planToCache).executedPlan, - tableName))) + val inMemoryRelation = InMemoryRelation( + sparkSession.sessionState.conf.useCompression, + sparkSession.sessionState.conf.columnBatchSize, + storageLevel, + sparkSession.sessionState.executePlan(planToCache).executedPlan, + tableName) + if (planToCache.conf.cboEnabled && planToCache.stats.rowCount.isDefined) { + inMemoryRelation.setStatsFromCachedPlan(planToCache) + } I have to make InMemoryRelation stateful to avoid breaking APIs..... — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#19864 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAEM96llEjZsyqac_xi9Nkks_2idfmgEks5s8YxWgaJpZM4QzBjk> .

CodingCat · 2017-12-03T06:17:04Z

@viirya yes, we can get more accurate stats later, however, the first stats is also important as it enables the user to pay less for the first run which writes cache.

The current implementation always chooses the most expensive plan in the first run, e.g. always resort to sortmergejoin instead of broadcastjoin even it is possible, CBO is actually disabled for any operator which locates in downstream of InMemoryRelation. Additionally, it makes execution plan inconsistent even for the same query over the same dataset. Of course, all of these issues happen in the first run.

IMHO, we have a chance to make it better, why not?

CodingCat · 2017-12-04T18:57:14Z

@viirya any thoughts?

viirya · 2017-12-05T15:06:35Z

sql/core/src/main/scala/org/apache/spark/sql/execution/CacheManager.scala

+        storageLevel,
+        sparkSession.sessionState.executePlan(planToCache).executedPlan,
+        tableName)
+      if (planToCache.conf.cboEnabled && planToCache.stats.rowCount.isDefined) {


Do we need to limit to those conditions? I think we can pass the stats into the created InMemoryRelation even the two conditions don't match.

the reason I put it here is that when we did not enable CBO, the stats in the underlying plan might be much smaller than the actual size in memory leading to the potential risk of OOM error.

The underlying cause is that without CBO enabled, the size of the plan is calculated with BaseRelation's sizeInBytes, but with CBO, we can have a more accurate estimation,

spark/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/LogicalRelation.scala

Lines 42 to 46 in 03fdc92

override def computeStats(): Statistics = {

catalogTable

.flatMap(_.stats.map(_.toPlanStats(output, conf.cboEnabled)))

.getOrElse(Statistics(sizeInBytes = relation.sizeInBytes))

}

spark/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala

Lines 370 to 381 in 03fdc92

def toPlanStats(planOutput: Seq[Attribute], cboEnabled: Boolean): Statistics = {

if (cboEnabled && rowCount.isDefined) {

val attrStats = AttributeMap(planOutput.flatMap(a => colStats.get(a.name).map(a -> _)))

// Estimate size as number of rows * row size.

val size = EstimationUtils.getOutputSize(planOutput, rowCount.get, attrStats)

Statistics(sizeInBytes = size, rowCount = rowCount, attributeStats = attrStats)

} else {

// When CBO is disabled or the table doesn't have other statistics, we apply the size-only

// estimation strategy and only propagate sizeInBytes in statistics.

Statistics(sizeInBytes = sizeInBytes)

}

}

When CBO is disabled, don't we just set the sizeInBytes to defaultSizeInBytes? Is it different than current statistics of first run?

no, if CBO is disabled, the relation's sizeInBytes is the file size

spark/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/HadoopFsRelation.scala

Line 85 in 5c3a1f3

override def sizeInBytes: Long = location.sizeInBytes

LogicalRelation uses the statistics from the relation only when there is no given catalogTable. In this case, it doesn't consider if CBO is enabled or not.

Only catalogTable considers CBO when computing its statistics in toPlanStats. It doesn't refer to relation's statistics, IIUC.

If a catalog table doesn't have statistics in its metadata, we will fill it with defaultSizeInBytes.

spark/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveStrategies.scala

Lines 121 to 134 in 326f1d6

val sizeInBytes = if (session.sessionState.conf.fallBackToHdfsForStatsEnabled) {

try {

val hadoopConf = session.sessionState.newHadoopConf()

val tablePath = new Path(table.location)

val fs: FileSystem = tablePath.getFileSystem(hadoopConf)

fs.getContentSummary(tablePath).getLength

} catch {

case e: IOException =>

logWarning("Failed to get table size from hdfs.", e)

session.sessionState.conf.defaultSizeInBytes

}

} else {

session.sessionState.conf.defaultSizeInBytes

}

@viirya you're right! Thanks for clearing the confusion

however, to prevent using relation's stats which can be much smaller than the in-memory size and lead to a potential OOM error, we should still have this condition here (we can remove cboEnabled though), right?

The statistics from relation is based on files size, will it easily cause OOM issue? I think in the cases other than cached query, we still use this relation's statistics. If this is an issue, doesn't it also affect the other cases?

that's true, it affects I believe....there is a similar discussion in #19743

viirya · 2017-12-05T15:07:33Z

sql/core/src/main/scala/org/apache/spark/sql/execution/columnar/InMemoryRelation.scala

 import org.apache.spark.storage.StorageLevel
 import org.apache.spark.util.LongAccumulator


 object InMemoryRelation {
+


Unnecessary change.

viirya · 2017-12-05T15:08:14Z

sql/core/src/main/scala/org/apache/spark/sql/execution/columnar/InMemoryRelation.scala

 import org.apache.spark.sql.execution.SparkPlan
+import org.apache.spark.sql.execution.datasources.LogicalRelation


Unused import.

SparkQA · 2017-12-05T17:19:48Z

Test build #84494 has finished for PR 19864 at commit b6a36be.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

CodingCat · 2017-12-05T17:33:28Z

thanks @viirya @cloud-fan and @hvanhovell, just addressed the comments and answered the question

SparkQA · 2017-12-05T17:44:42Z

Test build #84496 has finished for PR 19864 at commit 898f1b5.

This patch fails to build.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2017-12-05T18:28:47Z

Test build #84499 has finished for PR 19864 at commit 4c34701.

This patch fails to build.
This patch merges cleanly.
This patch adds no public classes.

cloud-fan · 2017-12-15T08:04:57Z

...core/src/test/scala/org/apache/spark/sql/execution/columnar/InMemoryColumnarQuerySuite.scala

+
+  test("SPARK-22673: InMemoryRelation should utilize existing stats of the plan to be cached") {
+    withSQLConf("spark.sql.cbo.enabled" -> "true") {
+      val workDir = s"${Utils.createTempDir()}/table1"


nit: use withTempDir

cloud-fan · 2017-12-15T08:06:22Z

...core/src/test/scala/org/apache/spark/sql/execution/columnar/InMemoryColumnarQuerySuite.scala

+      assert(inMemoryRelation.computeStats().sizeInBytes === 16)
+
+      // test of catalog table
+      val dfFromTable = spark.catalog.createTable("table1", workDir).cache()


nit: wrap with withTable, which will clean up the table automatically at the end.

cloud-fan · 2017-12-15T08:07:33Z

...core/src/test/scala/org/apache/spark/sql/execution/columnar/InMemoryColumnarQuerySuite.scala

+
+      // InMemoryRelation's stats should be updated after calculating stats of the table
+      spark.sql("ANALYZE TABLE table1 COMPUTE STATISTICS")
+      assert(inMemoryRelation2.computeStats().sizeInBytes === 16)


what happened here? InMemoryRelation.statsOfPlanToCache gets updated aotumatically?

it was a mistake here, AnalyzeTableCommand would actually force the table to be evaluated with count(), and it actually hits the longAccumulator's value,

fixed in the latest commit

SparkQA · 2017-12-18T00:59:16Z

Test build #85030 has finished for PR 19864 at commit b2d829b.

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

CodingCat · 2017-12-18T01:02:27Z

retest this please

HyukjinKwon · 2017-12-18T01:06:39Z

Let's hold retesting on. Seems globally failing.

SparkQA · 2017-12-18T03:52:16Z

Test build #85035 has finished for PR 19864 at commit b2d829b.

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

HyukjinKwon · 2017-12-18T08:41:09Z

retest this please

cloud-fan · 2017-12-18T08:55:34Z

...core/src/test/scala/org/apache/spark/sql/execution/columnar/InMemoryColumnarQuerySuite.scala

+    withSQLConf("spark.sql.cbo.enabled" -> "true") {
+      withTempDir { workDir =>
+        withTable("table1") {
+          val workDirPath = workDir.getAbsolutePath + "/table1"


seems you can use withTempPath, which just gives you a path string without creating it.

then we don't need to append /table1 to the path

cloud-fan · 2017-12-18T08:56:15Z

LGTM except one comment

SparkQA · 2017-12-18T11:27:44Z

Test build #85051 has finished for PR 19864 at commit b2d829b.

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

SparkQA · 2017-12-18T19:47:10Z

Test build #85067 has finished for PR 19864 at commit de2905c.

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

cloud-fan · 2017-12-19T02:38:13Z

...core/src/test/scala/org/apache/spark/sql/execution/columnar/InMemoryColumnarQuerySuite.scala

+          spark.sql("ANALYZE TABLE table1 COMPUTE STATISTICS")
+          val inMemoryRelation3 = spark.read.table("table1").cache().queryExecution.optimizedPlan.
+            collect { case plan: InMemoryRelation => plan }.head
+          assert(inMemoryRelation3.computeStats().sizeInBytes === 48)


missed this one, why does it have a different stats than the table cache stats 16?

because 16 is the exact in-memory size which is got by reading the accumulator's value after evaluating the RDD

48 is calculated by EstimationUtils:

spark/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/statsEstimation/EstimationUtils.scala

Lines 65 to 88 in bdb5e55

def getOutputSize(

attributes: Seq[Attribute],

outputRowCount: BigInt,

attrStats: AttributeMap[ColumnStat] = AttributeMap(Nil)): BigInt = {

// We assign a generic overhead for a Row object, the actual overhead is different for different

// Row format.

val sizePerRow = 8 + attributes.map { attr =>

if (attrStats.contains(attr)) {

attr.dataType match {

case StringType =>

// UTF8String: base + offset + numBytes

attrStats(attr).avgLen + 8 + 4

case _ =>

attrStats(attr).avgLen

}

} else {

attr.dataType.defaultSize

}

}.sum

// Output size can't be zero, or sizeInBytes of BinaryNode will also be zero

// (simple computation of statistics returns product of children).

if (outputRowCount > 0) outputRowCount * sizePerRow else 1

}

(8 + 4 (sum of average attribute length)) * 4

cloud-fan · 2017-12-19T13:52:46Z

thanks, merging to master!

CodingCat · 2017-12-19T14:52:24Z

thanks

…tion's size ## What changes were proposed in this pull request? as per discussion in apache#19864 (comment) the current HadoopFsRelation is purely based on the underlying file size which is not accurate and makes the execution vulnerable to errors like OOM Users can enable CBO with the functionalities in apache#19864 to avoid this issue This JIRA proposes to add a configurable factor to sizeInBytes method in HadoopFsRelation class so that users can mitigate this problem without CBO ## How was this patch tested? Existing tests Author: CodingCat <zhunansjtu@gmail.com> Author: Nan Zhu <nanzhu@uber.com> Closes apache#20072 from CodingCat/SPARK-22790.

…tion's size ## What changes were proposed in this pull request? as per discussion in #19864 (comment) the current HadoopFsRelation is purely based on the underlying file size which is not accurate and makes the execution vulnerable to errors like OOM Users can enable CBO with the functionalities in #19864 to avoid this issue This JIRA proposes to add a configurable factor to sizeInBytes method in HadoopFsRelation class so that users can mitigate this problem without CBO ## How was this patch tested? Existing tests Author: CodingCat <zhunansjtu@gmail.com> Author: Nan Zhu <nanzhu@uber.com> Closes #20072 from CodingCat/SPARK-22790. (cherry picked from commit ba891ec) Signed-off-by: gatorsmile <gatorsmile@gmail.com>

gatorsmile · 2018-01-23T19:15:21Z

sql/core/src/main/scala/org/apache/spark/sql/execution/columnar/InMemoryRelation.scala

@@ -60,7 +62,8 @@ case class InMemoryRelation(
    @transient child: SparkPlan,
    tableName: Option[String])(
    @transient var _cachedColumnBuffers: RDD[CachedBatch] = null,
-    val batchStats: LongAccumulator = child.sqlContext.sparkContext.longAccumulator)
+    val batchStats: LongAccumulator = child.sqlContext.sparkContext.longAccumulator,
+    statsOfPlanToCache: Statistics = null)


Why we set this to null?

eh...we do not have other options, it's more like a placeholder, since InMemoryRelation is created by CacheManager through apply() in companion object it's no harm here IMHO

## What changes were proposed in this pull request? This is a regression introduced by apache#19864 When we lookup cache, we should not carry the hint info, as this cache entry might be added by a plan having hint info, while the input plan for this lookup may not have hint info, or have different hint info. ## How was this patch tested? a new test. Author: Wenchen Fan <wenchen@databricks.com> Closes apache#20394 from cloud-fan/cache.

## What changes were proposed in this pull request? This is a regression introduced by #19864 When we lookup cache, we should not carry the hint info, as this cache entry might be added by a plan having hint info, while the input plan for this lookup may not have hint info, or have different hint info. ## How was this patch tested? a new test. Author: Wenchen Fan <wenchen@databricks.com> Closes #20394 from cloud-fan/cache. (cherry picked from commit 5b5447c) Signed-off-by: gatorsmile <gatorsmile@gmail.com>

CodingCat added 2 commits December 1, 2017 13:54

improve the doc for "spark.memory.offHeap.size"

b2fb1d2

fix

0971900

use cbo stats in inmemoryrelation

32f7c74

fix compilation issue

2082c0e

viirya reviewed Dec 2, 2017

View reviewed changes

CodingCat changed the title ~~[SPARK-22673][SQL] InMemoryRelation should utilize on-disk table stats whenever possible~~ [WIP][SPARK-22673][SQL] InMemoryRelation should utilize on-disk table stats whenever possible Dec 2, 2017

CodingCat changed the title ~~[WIP][SPARK-22673][SQL] InMemoryRelation should utilize on-disk table stats whenever possible~~ [SPARK-22673][SQL] InMemoryRelation should utilize on-disk table stats whenever possible Dec 2, 2017

CodingCat commented Dec 2, 2017

View reviewed changes

set stats in cachedmanager

bc70817

CodingCat changed the title ~~[SPARK-22673][SQL] InMemoryRelation should utilize on-disk table stats whenever possible~~ [SPARK-22673][SQL] InMemoryRelation should utilize existing stats whenever possible Dec 2, 2017

fix the typo

4650307

viirya reviewed Dec 5, 2017

View reviewed changes

address the comments

b6a36be

fix the stylistic issue

898f1b5

fix compilation issue

4c34701

fix the stylistic issue

8f912a3

cloud-fan reviewed Dec 15, 2017

View reviewed changes

address the issues in test

b2d829b

cloud-fan reviewed Dec 18, 2017

View reviewed changes

change to withTempPath

de2905c

cloud-fan reviewed Dec 19, 2017

View reviewed changes

asfgit closed this in ab7346f Dec 19, 2017

CodingCat deleted the SPARK-22673 branch December 19, 2017 17:07

CodingCat mentioned this pull request Dec 25, 2017

[SPARK-22790][SQL] add a configurable factor to describe HadoopFsRelation's size #20072

Closed

gatorsmile reviewed Jan 23, 2018

View reviewed changes

gatorsmile mentioned this pull request Jan 23, 2018

[SPARK-23195] [SQL] Keep the Hint of Cached Data #20368

Closed

cloud-fan mentioned this pull request Jan 25, 2018

[SPARK-23214][SQL] cached data should not carry extra hint info #20394

Closed

	override def computeStats(): Statistics = {
	catalogTable
	.flatMap(_.stats.map(_.toPlanStats(output, conf.cboEnabled)))
	.getOrElse(Statistics(sizeInBytes = relation.sizeInBytes))
	}

	def toPlanStats(planOutput: Seq[Attribute], cboEnabled: Boolean): Statistics = {
	if (cboEnabled && rowCount.isDefined) {
	val attrStats = AttributeMap(planOutput.flatMap(a => colStats.get(a.name).map(a -> _)))
	// Estimate size as number of rows * row size.
	val size = EstimationUtils.getOutputSize(planOutput, rowCount.get, attrStats)
	Statistics(sizeInBytes = size, rowCount = rowCount, attributeStats = attrStats)
	} else {
	// When CBO is disabled or the table doesn't have other statistics, we apply the size-only
	// estimation strategy and only propagate sizeInBytes in statistics.
	Statistics(sizeInBytes = sizeInBytes)
	}
	}

	val sizeInBytes = if (session.sessionState.conf.fallBackToHdfsForStatsEnabled) {
	try {
	val hadoopConf = session.sessionState.newHadoopConf()
	val tablePath = new Path(table.location)
	val fs: FileSystem = tablePath.getFileSystem(hadoopConf)
	fs.getContentSummary(tablePath).getLength
	} catch {
	case e: IOException =>
	logWarning("Failed to get table size from hdfs.", e)
	session.sessionState.conf.defaultSizeInBytes
	}
	} else {
	session.sessionState.conf.defaultSizeInBytes
	}

		import org.apache.spark.sql.execution.SparkPlan
		import org.apache.spark.sql.execution.datasources.LogicalRelation

	def getOutputSize(
	attributes: Seq[Attribute],
	outputRowCount: BigInt,
	attrStats: AttributeMap[ColumnStat] = AttributeMap(Nil)): BigInt = {
	// We assign a generic overhead for a Row object, the actual overhead is different for different
	// Row format.
	val sizePerRow = 8 + attributes.map { attr =>
	if (attrStats.contains(attr)) {
	attr.dataType match {
	case StringType =>
	// UTF8String: base + offset + numBytes
	attrStats(attr).avgLen + 8 + 4
	case _ =>
	attrStats(attr).avgLen
	}
	} else {
	attr.dataType.defaultSize
	}
	}.sum

	// Output size can't be zero, or sizeInBytes of BinaryNode will also be zero
	// (simple computation of statistics returns product of children).
	if (outputRowCount > 0) outputRowCount * sizePerRow else 1
	}

[SPARK-22673][SQL] InMemoryRelation should utilize existing stats whenever possible #19864

[SPARK-22673][SQL] InMemoryRelation should utilize existing stats whenever possible #19864

Conversation

CodingCat commented Dec 1, 2017

What changes were proposed in this pull request?

How was this patch tested?

CodingCat commented Dec 1, 2017

SparkQA commented Dec 1, 2017

SparkQA commented Dec 2, 2017

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

SparkQA commented Dec 2, 2017

SparkQA commented Dec 2, 2017

viirya commented Dec 3, 2017 via email

CodingCat commented Dec 3, 2017

CodingCat commented Dec 4, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

viirya Dec 6, 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

CodingCat Dec 6, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparkQA commented Dec 5, 2017

CodingCat commented Dec 5, 2017

SparkQA commented Dec 5, 2017

SparkQA commented Dec 5, 2017

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

SparkQA commented Dec 18, 2017

CodingCat commented Dec 18, 2017

HyukjinKwon commented Dec 18, 2017 • edited Loading

SparkQA commented Dec 18, 2017

HyukjinKwon commented Dec 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cloud-fan commented Dec 18, 2017

SparkQA commented Dec 18, 2017

SparkQA commented Dec 18, 2017

Choose a reason for hiding this comment

CodingCat Dec 19, 2017 • edited Loading

Choose a reason for hiding this comment

cloud-fan commented Dec 19, 2017

CodingCat commented Dec 19, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

viirya Dec 6, 2017 •

edited

Loading

CodingCat Dec 6, 2017 •

edited

Loading

HyukjinKwon commented Dec 18, 2017 •

edited

Loading

CodingCat Dec 19, 2017 •

edited

Loading