[Clustering] Check in new table properties #15985

lliiun-z · 2021-04-22T06:13:09Z

To enable clustering, we add three table properties.

clustered_by: the columns used for clustering.
cluster_count: the number of clusters.
distribution: the data distribution of each cluster column.

Test:

Added an integration test to ensure the syntax is accepted by Presto.

Test plan - (Please fill in how you tested your changes)

Please make sure your submission complies with our Development, Formatting, and Commit Message guidelines. Don't forget to follow our attribution guidelines for any code copied from other projects.

Fill in the release notes towards the bottom of the PR description.
See Release Notes Guidelines for details.

== RELEASE NOTES ==

General Changes
* ...
* ...

Hive Changes
* ...
* ...

If release note is NOT required, use:

== NO RELEASE NOTE ==

linux-foundation-easycla · 2021-04-22T06:13:11Z

✅ Grace Xin (56ae2a9)

❌ The commit (03d582e ,23a5a53f2a9d2b6ae34a5682b3561a8c010d104c ,5ec42b3cc67936c142cadbb07ad98183bfb30cfc ,5a740b675094e37b86e46cce6471da74d8104b15 ,bb071bd49b2575b172bdfa7f405140f02e1d2c4b ,6b981e286cbae4606202b596b96c792b108e108b ,3b60b7df68f53e250d924dcfe916dfd8d3309a13 ,b6d06f5854987a6cee4f08a3cc364778d3a3cbb6) is missing the User's ID, preventing the EasyCLA check. Consult GitHub Help to resolve.For further assistance with EasyCLA, please submit a support request ticket.

shixuan-fan

"[Clustering] Check in new table properties"

Besides comment, how about changing commit title to: Introduce clustering to hive table properties? Also, we probably don't need the Test part in commit message.

shixuan-fan · 2021-04-30T15:53:59Z

presto-hive/src/main/java/com/facebook/presto/hive/HiveTableProperties.java

+                                .collect(Collectors.toList())),
+                        value -> value),
+                new PropertyMetadata<>(
+                        CLUSTER_COUNT_PROPERTY,


We should use integerProperty for cluster count. You could refer to BUCKET_COUNT_PROPERTY as these two seem pretty similar.

The reason that we use an array is that we want to allow owners or us to specify the bucket count for each dimension column and remove the complicated logic to determine how many buckets for each dimension column. If later we find out such logic is simple and stable, we can use single integer for cluster count.

shixuan-fan · 2021-04-30T15:55:13Z

presto-hive/src/main/java/com/facebook/presto/hive/HiveTableProperties.java

+                new PropertyMetadata<>(
+                        DISTRIBUTION_PROPERTY,
+                        "Distributions of clustering columns",
+                        typeManager.getType(parseTypeSignature("array(varchar)")),


Looking at the test below, this seems to be the split values for different range clustering. This is actually quite tricky because:

What is the expected input when there are multiple clustering columns?

The type of this property depends on the type of input columns, which is not known at compile time

I think keeping it as an array(varchar) is okay because varchar is arguably the most flexible representation. But we probably would need a util class to be able to encode/decode the string from/to the actual Java class for engine processing. Also, we would need to specify in the property distribution about the expected inputs.

For cluster count should always be numbers. For distribution, you are right that the types are undetermined at compiling time.

shixuan-fan

"[Clustering] Expose clustering properties to hivebucketproperties object"

This commit itself is not a complete functionality so it is hard to give a proper commit title. Maybe we should squash them into one once the review is done? 🤔

shixuan-fan · 2021-04-30T16:08:59Z

presto-hive-metastore/src/main/java/com/facebook/presto/hive/HiveBucketProperty.java

+        } else if (bucketFunctionType.equals(HIVE_CLUSTERING)) {
+            checkArgument(clusteredBy.isPresent(), "Cluster columns must be present for bucket function type " + bucketFunctionType);
+            checkArgument(clusterCount.isPresent(), "Cluster numbers must be present for bucket function type " + bucketFunctionType);
+            checkArgument(bucketedBy.isEmpty(), "Bucket columns should not be present for bucket function type " + bucketFunctionType);


I guess clustering is compatible with sortedBy but want to call out here in case I'm not understanding correctly :)

@shixuan-fan, right! Clustering should be naturally compatible with SortedBy. I will add tests to confirm if it works.

shixuan-fan · 2021-04-30T16:16:52Z

presto-hive-metastore/src/main/java/com/facebook/presto/hive/HiveBucketProperty.java


    @JsonCreator
    public HiveBucketProperty(
            @JsonProperty("bucketedBy") List<String> bucketedBy,
            @JsonProperty("bucketCount") int bucketCount,
            @JsonProperty("sortedBy") List<SortingColumn> sortedBy,
            @JsonProperty("bucketFunctionType") BucketFunctionType bucketFunctionType,
-            @JsonProperty("types") Optional<List<Type>> types)
+            @JsonProperty("types") Optional<List<Type>> types,
+            @JsonProperty("clusteredBy") Optional<List<String>> clusteredBy,


Since we already have BucketFunctionType, do we still need to differentiate bucketedBy <-> clusteredBy and bucketCount <-> clusterCount? Logically they are doing the same thing, which is figuring out which columns should we bucket on and by how many buckets, and apply the bucket function as specified.

@shixuan-fan, actually it's tricky. bucketedBy in hive world has too strong assumption on hash bucketing. This assumption is also buried in other places in our codebase. So might be good to distinguish them.

shixuan-fan · 2021-04-30T16:18:10Z

presto-hive-metastore/src/main/java/com/facebook/presto/hive/HiveBucketProperty.java

@@ -85,6 +101,9 @@ public HiveBucketProperty(
                storageDescriptor.getNumBuckets(),
                sortedBy,
                HIVE_COMPATIBLE,
+                Optional.empty(),


I think we should introduce clustering related extraction from storage descriptor here as well.

Have talked with James before; we may do it later after we can store these information in partition property using thrift structure.

shixuan-fan · 2021-04-30T16:19:15Z

presto-hive/src/main/java/com/facebook/presto/hive/HiveTableProperties.java

+        return Optional.empty();
+    }
+
+    private static boolean doBucketing(List<String> bucketedBy)


I think this function and doClustering should be inlined. It is a bit too trivial.

Ok, it is my personal preference of readability. Not sure if Java compiler can do the automatical inlining; if not, I will change.

shixuan-fan

[Clustering] Expose clustering information to HivePartitioningHandle

I think we need to think carefully about how to decode the distribution part to be type compatible with because I imagine when doing the actual Z-order computing, the type of the column would be critical (e.g. 1 - 100 as integer would be very different from '1' - '100' as varchar).

shixuan-fan · 2021-04-30T16:27:10Z

presto-hive-metastore/src/main/java/com/facebook/presto/hive/HiveBucketProperty.java

@@ -116,7 +116,19 @@ public HiveBucketProperty(
    @JsonProperty
    public int getBucketCount()
    {
-        return bucketCount;
+        if (bucketFunctionType == HIVE_COMPATIBLE || bucketFunctionType == PRESTO_NATIVE) {


nit: how about a switch statement?

shixuan-fan · 2021-04-30T16:27:45Z

presto-hive-metastore/src/main/java/com/facebook/presto/hive/HiveBucketProperty.java

+        return getMergedClusterCount(clusterCount.get());
+    }
+
+    private static int getMergedClusterCount(List<Integer> clusterCount) {


Would you mind elaborating what we are trying to do here? I'm a bit puzzled 😂

E.g., The cluster count is given as a list [8, 8, 8] for three clustering columns; however, the total cluster count should be 8 * 8 * 8 for the table.

shixuan-fan · 2021-04-30T16:28:49Z

presto-hive/src/main/java/com/facebook/presto/hive/HiveClusteringBucketFunction.java

+    private final List<String> distribution;
+    private final List<Type> types;
+
+    // TODO: Add logic to generate Z-Order based on these parameters.


Do we plan to introduce more clustering function other than Z-Order, or we want to make Z-Order the only function for clustering?

In the implementation, the interface should provide a function as {int getClusterId()}. Z-Order should be one possible implementation for the interface.

shixuan-fan · 2021-04-30T16:30:20Z

presto-hive/src/main/java/com/facebook/presto/hive/HiveNodePartitioningProvider.java

@@ -68,6 +75,7 @@ public ConnectorBucketNodeMap getBucketNodeMap(ConnectorTransactionHandle transa
        HivePartitioningHandle handle = (HivePartitioningHandle) partitioningHandle;
        NodeSelectionStrategy nodeSelectionStrategy = getNodeSelectionStrategy(session);
        int bucketCount = handle.getBucketCount();
+


nit: remove?

highker

Early comments; still reviewing

highker · 2021-04-30T17:47:38Z

presto-hive-metastore/src/main/java/com/facebook/presto/hive/BucketFunctionType.java

@@ -15,6 +15,7 @@

 public enum BucketFunctionType
 {
+    HIVE_CLUSTERING,


Let's call it CLUSTERING or ZORDER_CLUSTERING given it is not a Hive concept

highker · 2021-04-30T17:51:55Z

presto-hive-metastore/src/main/java/com/facebook/presto/hive/HiveBucketProperty.java


    @JsonCreator
    public HiveBucketProperty(
            @JsonProperty("bucketedBy") List<String> bucketedBy,
            @JsonProperty("bucketCount") int bucketCount,
            @JsonProperty("sortedBy") List<SortingColumn> sortedBy,
            @JsonProperty("bucketFunctionType") BucketFunctionType bucketFunctionType,
-            @JsonProperty("types") Optional<List<Type>> types)
+            @JsonProperty("types") Optional<List<Type>> types,
+            @JsonProperty("clusteredBy") Optional<List<String>> clusteredBy,


@shixuan-fan, actually it's tricky. bucketedBy in hive world has too strong assumption on hash bucketing. This assumption is also buried in other places in our codebase. So might be good to distinguish them.

highker · 2021-04-30T17:53:01Z

presto-hive-metastore/src/main/java/com/facebook/presto/hive/HiveBucketProperty.java

+    private final Optional<List<String>> clusteredBy;
+    private final Optional<List<Integer>> clusterCount;
+    private final Optional<List<String>> distribution;


no need to be Optional I guess. empty list can indicate there is no clustering. Same for bucketing.

highker · 2021-04-30T17:53:34Z

presto-hive-metastore/src/main/java/com/facebook/presto/hive/HiveBucketProperty.java

        if (bucketFunctionType.equals(PRESTO_NATIVE)) {
            checkArgument(types.isPresent(), "Types must be present for bucket function type " + bucketFunctionType);
            checkArgument(types.get().size() == bucketedBy.size(), "The sizes of bucketedBy and types should match");
+        } else if (bucketFunctionType.equals(HIVE_CLUSTERING)) {


We usually put else if in the new line

highker · 2021-04-30T17:53:54Z

presto-hive-metastore/src/main/java/com/facebook/presto/hive/HiveBucketProperty.java

+        } else if (bucketFunctionType.equals(HIVE_CLUSTERING)) {
+            checkArgument(clusteredBy.isPresent(), "Cluster columns must be present for bucket function type " + bucketFunctionType);
+            checkArgument(clusterCount.isPresent(), "Cluster numbers must be present for bucket function type " + bucketFunctionType);
+            checkArgument(bucketedBy.isEmpty(), "Bucket columns should not be present for bucket function type " + bucketFunctionType);


highker · 2021-04-30T18:01:36Z

presto-hive/src/main/java/com/facebook/presto/hive/HiveTableProperties.java

+        return Optional.empty();
+    }
+
+    private static boolean doBucketing(List<String> bucketedBy)


highker · 2021-04-30T19:00:00Z

presto-hive/src/main/java/com/facebook/presto/hive/HiveTableProperties.java

@@ -47,6 +47,9 @@
    public static final String BUCKETED_BY_PROPERTY = "bucketed_by";
    public static final String BUCKET_COUNT_PROPERTY = "bucket_count";
    public static final String SORTED_BY_PROPERTY = "sorted_by";
+    public static final String CLUSTERED_BY_PROPERTY = "clustered_by";
+    public static final String CLUSTER_COUNT_PROPERTY = "cluster_count";
+    public static final String DISTRIBUTION_PROPERTY = "distribution";


Let's maybe remove this from table properties at the moment. Kinda wanna make it hidden from users. Let me think about how to do that

As discussed offline, let's use another API to store and read distribution to hide it from end users.

From the discussion, I remember that API is not ready. Let me confirm with Suketu.

highker · 2021-05-05T05:06:51Z

presto-hive/src/main/java/com/facebook/presto/hive/HiveTableProperties.java

+                        typeManager.getType(parseTypeSignature("array(varchar)")),
+                        List.class,
+                        ImmutableList.of(),
+                        false,


Make it hidden before we find a way to store the distribution

highker · 2021-05-05T05:12:56Z

presto-hive-metastore/src/main/java/com/facebook/presto/hive/HiveBucketProperty.java

+            @JsonProperty("distribution") Optional<List<String>> distribution
+            )


put ) to the previous line

highker · 2021-05-05T05:15:27Z

presto-hive-metastore/src/main/java/com/facebook/presto/hive/HiveBucketProperty.java

+
+

We disallow two consecutive empty lines

highker · 2021-05-05T05:20:01Z

presto-hive/src/main/java/com/facebook/presto/hive/HiveMetadata.java

@@ -1007,12 +1007,16 @@ public ConnectorTableHandle createTemporaryTable(ConnectorSession session, List<
            List<String> partitionColumns = partitioning.getPartitionColumns();
            BucketFunctionType bucketFunctionType = partitioningHandle.getBucketFunctionType();
            switch (bucketFunctionType) {
+                case HIVE_CLUSTERING:


The function is to create temp table. We should actually throw if we see a clustering when writing to temp tables.

highker · 2021-05-05T05:23:40Z

presto-hive/src/main/java/com/facebook/presto/hive/HiveTableProperties.java

+            List<String> clusteredBy,
+            List<Integer> clusterCount,
+            List<String> distribution)


We should check

the clusteredBy size is the same as clusterCount

clusterCount has all positive integers that are all 2^n. Also it should between something like 0 and 2^16

the multiplication of all cluster numbers should be <= 2^16

Right. Will check.

highker · 2021-05-05T05:38:15Z

presto-hive/src/test/java/com/facebook/presto/hive/TestHiveClustering.java

+    public void testCreateTableAsSelect()
+    {
+        String query = (
+                "CREATE TABLE hive_bucketed.tpch_bucketed.customer_copied \n" +
+                        " WITH (" +
+                        "    format = 'TEXTFILE',\n" +
+                        "    clustered_by = array['custkey'], \n" +
+                        "    cluster_count = array[11], \n" +
+                        "    distribution = array['150', '300', '450', '600', '750', '900', '1050', '1200', '1350']) \n" +
+                        " AS SELECT * FROM customer");
+
+        MaterializedResult result = computeActual(query);
+        assertEquals(result.getOnlyValue(), 1500L);
+
+        query = "SELECT COUNT(*) FROM hive_bucketed.tpch_bucketed.customer_copied";
+        result = computeActual(query);
+        assertEquals(result.getOnlyValue(), 1500L);
+
+        query = "SELECT custkey, COUNT(*) FROM hive_bucketed.tpch_bucketed.customer_copied GROUP BY custkey ORDER BY custkey LIMIT 1 ";
+        result = computeActual(query);
+        assertEquals(result.getRowCount(), 1L);
+        assertEquals(result.getMaterializedRows().get(0).getField(1), 1L);
+    }
+
+    @Test
+    public void testInsert()


Merge these two tests together.

highker · 2021-05-05T05:40:23Z

presto-hive/src/test/java/com/facebook/presto/hive/TestHiveClustering.java

+        result = computeActual(query);
+        assertEquals(result.getOnlyValue(), 3000L);
+
+        query = "SELECT custkey, COUNT(*) FROM hive_bucketed.tpch_bucketed.customer_copied GROUP BY custkey ORDER BY custkey LIMIT 1 ";


An interesting test would be creating two tables: one with clustering and one without. Then use the same select query with some proper filters against these two tables. Then we should expect the one against clustering table will read fewer rows and data.

How to check the query stats to get the rows and data read?

highker · 2021-05-05T05:42:44Z

presto-hive-metastore/src/main/java/com/facebook/presto/hive/HiveBucketProperty.java

@@ -43,7 +43,7 @@
    private final Optional<List<Type>> types;
    private final Optional<List<String>> clusteredBy;
    private final Optional<List<Integer>> clusterCount;
-    private final Optional<List<String>> distribution;
+    private final Optional<List<Object>> distribution;


Well, as discussed offline, we will need a good data structure to represent distribution. ColumnStatistics is a good example. We likely could make it a thrift serializable class. This part needs some brainstorming. Let's chat offline.

highker · 2021-05-05T05:47:05Z

presto-hive/src/main/java/com/facebook/presto/hive/clustering/IntervalExtractor.java

+import java.util.Map;
+import java.util.Optional;
+
+public final class IntervalExtractor


Let's chat offline on the distribution design and how to store it. It is fairly critical I think

highker · 2021-05-05T05:48:56Z

presto-hive/src/test/java/com/facebook/presto/hive/TestHiveUtil.java

+                "rO0ABXNyABFqYXZhLmxhbmcuSW50ZWdlchLioKT3gYc4AgABSQAFdmFsdWV4cgAQamF2YS5sYW5nLk51bWJlcoaslR0LlOCLAgAAeHAAAAAB",
+                "c3IAEWphdmEubGFuZy5JbnRlZ2VyEuKgpPeBhzgCAAFJAAV2YWx1ZXhyABBqYXZhLmxhbmcuTnVtYmVyhqyVHQuU4IsCAAB4cAAAAAI=",
+                "c3IAEWphdmEubGFuZy5JbnRlZ2VyEuKgpPeBhzgCAAFJAAV2YWx1ZXhyABBqYXZhLmxhbmcuTnVtYmVyhqyVHQuU4IsCAAB4cAAAAAM=",
+                "c3IAEGphdmEubGFuZy5Eb3VibGWAs8JKKWv7BAIAAUQABXZhbHVleHIAEGphdmEubGFuZy5OdW1iZXKGrJUdC5TgiwIAAHhwP/GZmZmZmZo=",
+                "c3IAEGphdmEubGFuZy5Eb3VibGWAs8JKKWv7BAIAAUQABXZhbHVleHIAEGphdmEubGFuZy5OdW1iZXKGrJUdC5TgiwIAAHhwQAGZmZmZmZo=",
+                "c3IAEGphdmEubGFuZy5Eb3VibGWAs8JKKWv7BAIAAUQABXZhbHVleHIAEGphdmEubGFuZy5OdW1iZXKGrJUdC5TgiwIAAHhwQApmZmZmZmY=",
+                "dAABYQ==",
+                "dAABYg==",
+                "dAABeA=="));


wow; definitely we won't expose this to end users lol

viczhang861

is this feature about partition data to different clusters?

viczhang861 · 2021-05-17T19:57:42Z

presto-hive/src/main/java/com/facebook/presto/hive/HiveClusteringBucketFunction.java

+    // TODO: Return Z-Order Address.
+    public int getBucket(Page page, int position)
+    {
+        return clustering.getCluster(page, position);


This method is supposed to calculate bucket number, is cluster id similar to bucket id?

What do you think of adding getPartitionFunction to interface ConnectorNodePartitioningProvider
and adding ClusterPartitionFunction that implements PartitionFunction

To enable clustering, we add three table properties. 1. clustered_by: the columns used for clustering. 2. cluster_count: the number of clusters. 3. distribution: the data distribution of each cluster column. Test: 1. Added an integration test to ensure the syntax is accepted by Presto.

To expose these three parameters, we 1. Create a new BucketFunctionType: HIVE_CLUSTERING. 2. Update the logic/constructor in HiveBucketProperties. 3. Update the usage of HiveBucketProperties constructor to pass empty values for clustering parameters. Test: This diff should not produce any side effect, it only update the APIs of the class.

This diff mainly add the z-order logic, which takes the clusterCount, clusteredBy, distribution, and types to construct the MortonCode object, which translate the dimension values into clusterId, which is returned as a bucket id. TESTS: Add unit tests.

So far storagedescriptor is not updated to have clustering paramters. Therefore, some checks will fail when we do clustering. To solve this problem, we read the clustering paramters from table parameters.

These information will be used in query like DESC or SHOW CREATE TABLE. TEST: Ran local integration test and tested in test cluster.

…scriptor TEST: 1. Add Unit test. 2. Tested in local and testing cluster.

1. Integrate with our own z-order library. 2. Fixed a few bugs

This is used to reduce the cpu time for creating page source.

lliiun-z requested review from viczhang861, highker, NikhilCollooru and shixuan-fan April 27, 2021 23:01

shixuan-fan reviewed Apr 30, 2021

View reviewed changes

highker reviewed Apr 30, 2021

View reviewed changes

highker self-requested a review April 30, 2021 19:55

highker reviewed May 5, 2021

View reviewed changes

lliiun-z force-pushed the hive_clustering_features branch 2 times, most recently from c462bc1 to 534cf5e Compare May 7, 2021 23:46

viczhang861 reviewed May 17, 2021

View reviewed changes

lliiun-z force-pushed the hive_clustering_features branch from 534cf5e to 1b6ee6c Compare May 18, 2021 21:47

lliiun-z force-pushed the hive_clustering_features branch 3 times, most recently from 0d84fd4 to 0b9aa26 Compare June 14, 2021 16:14

lliiun-z requested review from highker and shixuan-fan June 15, 2021 23:11

lliiun-z force-pushed the hive_clustering_features branch from 0b9aa26 to bb717aa Compare June 17, 2021 18:26

lliiun-z force-pushed the hive_clustering_features branch 2 times, most recently from feb5e36 to cbaa6af Compare July 19, 2021 21:47

lliiun-z added 7 commits August 3, 2021 15:15

Add Z-order logic

5ec42b3

This diff mainly add the z-order logic, which takes the clusterCount, clusteredBy, distribution, and types to construct the MortonCode object, which translate the dimension values into clusterId, which is returned as a bucket id. TESTS: Add unit tests.

Read clustering parameters before storagedescriptor is updated

5a740b6

So far storagedescriptor is not updated to have clustering paramters. Therefore, some checks will fail when we do clustering. To solve this problem, we read the clustering paramters from table parameters.

Export bucketing/clustering information correctly to table metadata

bb071bd

These information will be used in query like DESC or SHOW CREATE TABLE. TEST: Ran local integration test and tested in test cluster.

Serialize/Deserialize clustering information into/out from storage de…

6b981e2

…scriptor TEST: 1. Add Unit test. 2. Tested in local and testing cluster.

Refactor on Morton Code

3b60b7d

1. Integrate with our own z-order library. 2. Fixed a few bugs

gxin@fb.com and others added 2 commits August 3, 2021 15:15

Support exclusively positive integer columns

56ae2a9

Cache configuration and file systems

b6d06f5

This is used to reduce the cpu time for creating page source.

lliiun-z force-pushed the hive_clustering_features branch from cbaa6af to b6d06f5 Compare August 3, 2021 22:15

highker removed their request for review December 16, 2021 21:18

lliiun-z closed this Sep 6, 2022

		@JsonProperty("distribution") Optional<List<String>> distribution
		)

[Clustering] Check in new table properties #15985

[Clustering] Check in new table properties #15985

Conversation

lliiun-z commented Apr 22, 2021

linux-foundation-easycla bot commented Apr 22, 2021 • edited

shixuan-fan 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

shixuan-fan 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

shixuan-fan 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

highker 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

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

viczhang861 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

linux-foundation-easycla bot commented Apr 22, 2021 •

edited