Enable adding new segments to a upsert-enabled realtime table #6567

chenboat · 2021-02-09T23:53:20Z

Description

Currently segment upload is only applicable for offline Pinot tables. This PR opens the segment upload to Pinot realtime tables too. Segment upload to realtime tables makes bootstrapping of a realtime table much faster because the segments can now be computed outside of Pinot. This features allows Pinot realtime tables with longer retention (e.g., Pinot upsert enable table) to be bootstrapped in a much efficient manner.

Design doc: https://docs.google.com/document/d/1STYxZsUYGcYrzHdmOBymQ1NkpxuEzLa4OTiw4mGHiKk/edit#

Upgrade Notes

Does this PR prevent a zero down-time upgrade? (Assume upgrade order: Controller, Broker, Server, Minion)

Yes (Please label as backward-incompat, and complete the section below on Release Notes)

Does this PR fix a zero-downtime upgrade introduced earlier?

Yes (Please label this as backward-incompat, and complete the section below on Release Notes)

Does this PR otherwise need attention when creating release notes? Things to consider:

New configuration options
Deprecation of configurations
Signature changes to public methods/interfaces
New plugins added or old plugins removed

Yes (Please label this PR as release-notes and complete the section on Release Notes)

Release Notes

If you have tagged this as either backward-incompat or release-notes,
you MUST add text here that you would like to see appear in release notes of the
next release.

If you have a series of commits adding or enabling a feature, then
add this section only in final commit that marks the feature completed.
Refer to earlier release notes to see examples of text

Documentation

If you have introduced a new feature or configuration, please add it to the documentation as well.
See https://docs.pinot.apache.org/developers/developers-and-contributors/update-document

codecov-io · 2021-02-10T00:50:52Z

Codecov Report

Merging #6567 (38cca2c) into master (1beaab5) will decrease coverage by 0.55%.
The diff coverage is 63.15%.

@@            Coverage Diff             @@
##           master    #6567      +/-   ##
==========================================
- Coverage   66.44%   65.89%   -0.56%     
==========================================
  Files        1075     1403     +328     
  Lines       54773    67986   +13213     
  Branches     8168     9836    +1668     
==========================================
+ Hits        36396    44797    +8401     
- Misses      15700    19997    +4297     
- Partials     2677     3192     +515

Flag	Coverage Δ
unittests	`65.89% <63.15%> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...e/pinot/broker/api/resources/PinotBrokerDebug.java	`0.00% <0.00%> (-79.32%)`	⬇️
...pinot/broker/api/resources/PinotClientRequest.java	`0.00% <0.00%> (-27.28%)`	⬇️
...ot/broker/broker/AllowAllAccessControlFactory.java	`85.71% <ø> (-14.29%)`	⬇️
.../helix/BrokerUserDefinedMessageHandlerFactory.java	`33.96% <0.00%> (-32.71%)`	⬇️
...ker/routing/instanceselector/InstanceSelector.java	`100.00% <ø> (ø)`
...ava/org/apache/pinot/client/AbstractResultSet.java	`66.66% <ø> (+9.52%)`	⬆️
...n/java/org/apache/pinot/client/BrokerResponse.java	`100.00% <ø> (ø)`
.../main/java/org/apache/pinot/client/Connection.java	`35.55% <ø> (-13.29%)`	⬇️
...org/apache/pinot/client/DynamicBrokerSelector.java	`82.85% <ø> (+10.12%)`	⬆️
...n/java/org/apache/pinot/client/ExecutionStats.java	`68.88% <ø> (ø)`
... and 1290 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 53c2a3c...38cca2c. Read the comment docs.

mcvsubbu

What is the validation done to make sure that the segments in the realtime table do not overlap data with the newly loaded ones?

...ava/org/apache/pinot/controller/api/resources/PinotSegmentUploadDownloadRestletResource.java

Jackie-Jiang

How do we manage the stream offset if we upload a segment to the realtime table? Also, how about the segment name format?
We need to have an integration test to ensure the PinotLLCRealtimeSegmentManager can work properly on uploaded segments.

...ava/org/apache/pinot/controller/api/resources/PinotSegmentUploadDownloadRestletResource.java

Jackie-Jiang · 2021-02-10T02:38:40Z

...ntroller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java

+
+    if (isRealtimeOnlyTable(tableName)) {
+      tableNameWithType = TableNameBuilder.REALTIME.tableNameWithType(tableName);
+      instancePartitionsType = InstancePartitionsType.CONSUMING;


This should be COMPLETED

In an upsert enabled LLC realtime table, all segments of the same partition are colocated on the same server -- consuming or completed. So it is fine to use CONSUMING as the InstancePartitionsType.

In addition, the current RealtimeSegmentAssignment.assignSegment(..) method, it is hardcoded explicitly to assign new segments to CONSUMING instance partition.
https://github.com/apache/incubator-pinot/blob/3b7093039469b2a00269f747543c6c29c8f1801f/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/RealtimeSegmentAssignment.java#L102

I agree with Jackie that it is counter-intuitive. Add a comment here reasoning why it should be set to CONSUMING. If we enable uploading realtime segments to non-upsert tables, at least we will know why/what we will need to fix

added comments about why we chose CONSUMING today. Left TODO for further improvement.

snleee · 2021-02-10T04:14:42Z

@chenboat Can you open up the access for the linked design doc?

chenboat · 2021-02-10T19:19:25Z

How do we manage the stream offset if we upload a segment to the realtime table? Also, how about the segment name format?
We need to have an integration test to ensure the PinotLLCRealtimeSegmentManager can work properly on uploaded segments.

Please refer to the comments I added with respect to Subbu's questions. For upsert tables, there should be validation that the offset of uploaded segments will not affect the consuming segment's offset. This is inline with the goals of segment upload — which are mainly about bootstrapping historical data.

The segment name format I propose conforms to the LLC name format with the following parts. tableName(string)partitionId(int)_sequenceNumber(int)_creationTime(string). The main difference is that the creationTime string now does not need to an actual timestamp but rather any unique string (e.g., timestamp + an uuid).

Yes. I will add an integration test.

...ava/org/apache/pinot/controller/api/resources/PinotSegmentUploadDownloadRestletResource.java

pinot-controller/src/main/java/org/apache/pinot/controller/api/upload/ZKOperator.java

...ntroller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java

pinot-common/src/main/java/org/apache/pinot/common/utils/fetcher/SegmentFetcherFactory.java

xiangfu0 · 2021-03-13T09:31:11Z

pinot-common/src/main/java/org/apache/pinot/common/utils/CommonConstants.java

@@ -358,7 +358,7 @@
  public static class Segment {
    public static class Realtime {
      public enum Status {
-        IN_PROGRESS, DONE
+        IN_PROGRESS, DONE, UPLOAD


Do you think if we need to have separated status for UPLOAD_UPSERT and UPLOAD_APPEND?

Currently UPLOAD is only applicable to upsert enable tables. Is there an effort to expand the scope? for upsert table, UPLOAD_APPEND is not needed.

pinot-integration-tests/src/test/resources/upsert_table_test.schema

...core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeTableDataManager.java

...ava/org/apache/pinot/controller/helix/core/assignment/segment/RealtimeSegmentAssignment.java

xiangfu0 · 2021-03-13T09:52:09Z

...ntroller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java

+  public boolean isUpsertTable(String tableName) {
+    if (hasOfflineTable(tableName))
+      return false;
+    if (!hasRealtimeTable(tableName))


is this if-condition redundant?

It is not redundant because a table which does not exist may reach here.

...ntroller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java

xiangfu0 · 2021-03-13T10:00:36Z

...ava/org/apache/pinot/controller/api/resources/PinotSegmentUploadDownloadRestletResource.java

@@ -245,21 +245,28 @@ private SuccessResponse uploadSegment(@Nullable String tableName, FormDataMultiP
        LOGGER.info("Uploading a segment {} to table: {}, push type {}, (Derived from segment metadata)", segmentName, tableName, uploadType);
      }

-      String offlineTableName = TableNameBuilder.OFFLINE.tableNameWithType(rawTableName);
+      String tableNameWithType;
+      if (_pinotHelixResourceManager.isUpsertTable(rawTableName)) {


Shall we throw an exception on the non-upsert realtime table?

This logic seems a little strange. Does this mean that we cannot upload segment to the offline part of an upsert table ? Or that offline part does not exist for an upsert table? What if we introduce this part later?
Instead, I suggest that we include the type query-part as well in the url. If the type is not present, we assume it is OFFLINE. If it is present, we treat it accordingly, but throw an exception if it is realtime type but upsert is not turned on

yes it is a good idea to divide into realtime and offline cases rather than upsert vs non-upsert case. Added the table type parameters to the REST end point too.

pinot-common/src/main/java/org/apache/pinot/common/utils/CommonConstants.java

mcvsubbu · 2021-03-16T22:35:21Z

...ava/org/apache/pinot/controller/api/resources/PinotSegmentUploadDownloadRestletResource.java

@@ -245,21 +245,28 @@ private SuccessResponse uploadSegment(@Nullable String tableName, FormDataMultiP
        LOGGER.info("Uploading a segment {} to table: {}, push type {}, (Derived from segment metadata)", segmentName, tableName, uploadType);
      }

-      String offlineTableName = TableNameBuilder.OFFLINE.tableNameWithType(rawTableName);
+      String tableNameWithType;
+      if (_pinotHelixResourceManager.isUpsertTable(rawTableName)) {


This logic seems a little strange. Does this mean that we cannot upload segment to the offline part of an upsert table ? Or that offline part does not exist for an upsert table? What if we introduce this part later?
Instead, I suggest that we include the type query-part as well in the url. If the type is not present, we assume it is OFFLINE. If it is present, we treat it accordingly, but throw an exception if it is realtime type but upsert is not turned on

pinot-common/src/main/java/org/apache/pinot/common/utils/CommonConstants.java

pinot-common/src/main/java/org/apache/pinot/common/utils/fetcher/SegmentFetcherFactory.java

...ava/org/apache/pinot/controller/helix/core/assignment/segment/RealtimeSegmentAssignment.java

...core/src/main/java/org/apache/pinot/core/data/manager/realtime/RealtimeTableDataManager.java

yupeng9

LGTM

pinot-common/src/main/java/org/apache/pinot/common/utils/fetcher/SegmentFetcherFactory.java

Jackie-Jiang

Please take a look at PinotLLCRealtimeSegmentManager and ensure it works on uploaded segments:

Segment name for uploaded segment does not follow the llc segment name convention
Exclude uploaded segment from the processing

pinot-common/src/main/java/org/apache/pinot/common/utils/fetcher/SegmentFetcherFactory.java

...ava/org/apache/pinot/controller/api/resources/PinotSegmentUploadDownloadRestletResource.java

pinot-controller/src/main/java/org/apache/pinot/controller/api/upload/ZKOperator.java

...ava/org/apache/pinot/controller/helix/core/assignment/segment/RealtimeSegmentAssignment.java

chenboat · 2021-03-22T21:05:44Z

A new integration test UpsertTableSegmentUploadIntegrationTest is added to verify the uploaded segment is added to a realtime upsert enabled table correctly. The segment name used in the test does not follow llc segment convention but it is loaded successfully. The query result is good.

I do quite understand why we should "Exclude uploaded segment from the processing". During segment load, the upsert table metadata should be processed based on the loaded segment, right?

Please take a look at PinotLLCRealtimeSegmentManager and ensure it works on uploaded segments:

Segment name for uploaded segment does not follow the llc segment name convention

Exclude uploaded segment from the processing

Jackie-Jiang · 2021-03-23T19:22:56Z

A new integration test UpsertTableSegmentUploadIntegrationTest is added to verify the uploaded segment is added to a realtime upsert enabled table correctly. The segment name used in the test does not follow llc segment convention but it is loaded successfully. The query result is good.

I do quite understand why we should "Exclude uploaded segment from the processing". During segment load, the upsert table metadata should be processed based on the loaded segment, right?

Please take a look at PinotLLCRealtimeSegmentManager and ensure it works on uploaded segments:

Segment name for uploaded segment does not follow the llc segment name convention

Exclude uploaded segment from the processing

I took another look at PinotLLCRealtimeSegmentManager and seems non-LLC segments are already excluded (because it might co-exist with HLC segments before). We cannot include them during metadata processing because it relies on the segment name format.

For the issue within the processExistingSegment(), for uploading the new segment it won't hit that code path, but for refreshing the existing segment it won't set the fields properly.

If possible, I would recommend setting up an actual table, upload some segments and keep it running for some time to ensure all components can properly work on the uploaded segments

chenboat · 2021-03-23T20:45:20Z

A new integration test UpsertTableSegmentUploadIntegrationTest is added to verify the uploaded segment is added to a realtime upsert enabled table correctly. The segment name used in the test does not follow llc segment convention but it is loaded successfully. The query result is good.
I do quite understand why we should "Exclude uploaded segment from the processing". During segment load, the upsert table metadata should be processed based on the loaded segment, right?

Please take a look at PinotLLCRealtimeSegmentManager and ensure it works on uploaded segments:

Segment name for uploaded segment does not follow the llc segment name convention

Exclude uploaded segment from the processing

I took another look at PinotLLCRealtimeSegmentManager and seems non-LLC segments are already excluded (because it might co-exist with HLC segments before). We cannot include them during metadata processing because it relies on the segment name format.

For the issue within the processExistingSegment(), for uploading the new segment it won't hit that code path, but for refreshing the existing segment it won't set the fields properly.

If possible, I would recommend setting up an actual table, upload some segments and keep it running for some time to ensure all components can properly work on the uploaded segments

The integration test I added uploads a segment to a LLC table with all Pinot components up. The test shows the query can be run successfully on a Pinot upsert table with correct query results returned. It also verifies the idealstate. I can add more verification to the test such as more metadata check and more segments uploads. I prefer Pinot integration tests than manual tests.

for processExistingSegment(), I think we should leave it to another PR and let this PR focus on adding new segments. For now, this PR will reject upload segments of the same name. Notice that for our Upsert table use case, upload new segments would be good enough. I will have a follow up PR for refreshing existing segments.

Jackie-Jiang

LGTM otherwise.

I would still recommend running some long lasting manual test because integration test won't cover the periodic tasks running in controller. Also, in the integration test there is no event pushing to kafka, so we don't test the case where CONSUMING, COMPLETED and UPLOADED segments co-exist case

pinot-common/src/main/java/org/apache/pinot/common/utils/fetcher/SegmentFetcherFactory.java

pinot-common/src/main/java/org/apache/pinot/common/utils/FileUploadDownloadClient.java

...ava/org/apache/pinot/controller/api/resources/PinotSegmentUploadDownloadRestletResource.java

...ntroller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java

pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/util/ZKMetadataUtils.java

Jackie-Jiang · 2021-03-24T17:42:22Z

For the new integration test, it would be good if we push some events into kafka as well and make the commit threshold low so that we have CONSUMING, DONE, UPLOADED segments at the same time. Also, we can run the RealtimeSegmentValidationManager and see if it handles UPLOADED segments properly

chenboat · 2021-03-26T00:23:40Z

For the new integration test, it would be good if we push some events into kafka as well and make the commit threshold low so that we have CONSUMING, DONE, UPLOADED segments at the same time. Also, we can run the RealtimeSegmentValidationManager and see if it handles UPLOADED segments properly

Added the Kafka data ingestion to the integration test. Also add more segments (now 2 realtime segments and 3 uploaded segments are involved in the tests). Discovered a bug in the previous version.

Revised the integration test so that it runs the RealtimeSegmentValidationManager every 1 second and verifies the query results and idealstate are still valid. @Jackie-Jiang

…loaded: 2 realtime and 3 offline.

…ments.

mcvsubbu · 2021-04-02T18:06:08Z

pinot-common/src/main/java/org/apache/pinot/common/utils/CommonConstants.java

+        // Means the segment is completed and sealed by some Pinot server and uploaded
+        DONE,
+        // Means the segment is uploaded to a Pinot controller by an external party
+        UPLOADED


Should we take this opportunity to add two more enums: CONSUMING and COMPLETED (these are the terms used elsewhere), and slowly obsolete the IN_PROGRESS and DONE. For now, they will be treated equivalent to their counterpart.

You should only add enums at the end. But anyways, this is a deeper change. I was just asking of the other reviewers. Let us revert it and stick to your PR for now.
@npawar and @Jackie-Jiang may have an opinion. We can discuss separately.

I feel CONSUMING and COMPLETED is more clear, and consistent with the term we used in the segment assignment strategy

Removed the two enums.

@Jackie-Jiang agreed, just that it will need a migration of the existing metadata and/or code to scan both. Best to keep it out of this PR. @chenboat will be great if you can file an issue for this.

#6777 filed for this issue.

pinot-common/src/main/java/org/apache/pinot/common/utils/CommonConstants.java

...ava/org/apache/pinot/controller/api/resources/PinotSegmentUploadDownloadRestletResource.java

pinot-controller/src/main/java/org/apache/pinot/controller/api/upload/ZKOperator.java

mcvsubbu · 2021-04-02T20:14:20Z

...ntroller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java

+
+    if (isRealtimeOnlyTable(tableName)) {
+      tableNameWithType = TableNameBuilder.REALTIME.tableNameWithType(tableName);
+      instancePartitionsType = InstancePartitionsType.CONSUMING;


I agree with Jackie that it is counter-intuitive. Add a comment here reasoning why it should be set to CONSUMING. If we enable uploading realtime segments to non-upsert tables, at least we will know why/what we will need to fix

pinot-core/src/main/java/org/apache/pinot/core/util/SegmentUtils.java

…nConstants.java Co-authored-by: Subbu Subramaniam <mcvsubbu@users.noreply.github.com>

mcvsubbu · 2021-04-12T18:32:39Z

pinot-controller/src/main/java/org/apache/pinot/controller/api/upload/ZKOperator.java

-    LOGGER.info("Segment {} from table {} already exists, refreshing if necessary", segmentName, rawTableName);
+    // TODO Allow segment refreshing for realtime tables.
+    if (TableNameBuilder.isRealtimeTableResource(tableNameWithType)) {
+      LOGGER.info("Segment {} from table {} already exists, refreshing if necessary", segmentName, tableNameWithType);


Log line is misleading right? We are actually throwing an exception below. I think this log line belongs after line 84. It may be useful to have another log line here that says we do not support refresing existing segment for realtime table.

Moved the log line after the if block.

jtao15 · 2021-06-15T18:12:25Z

@chenboat Hi, can you open the read permission of the design document?

nisheet195 · 2021-09-02T19:02:14Z

+1.
@chenboat Can you make the design doc attached in the PR public

chenboat requested a review from Jackie-Jiang February 9, 2021 23:53

mcvsubbu reviewed Feb 10, 2021

View reviewed changes

...ava/org/apache/pinot/controller/api/resources/PinotSegmentUploadDownloadRestletResource.java Outdated Show resolved Hide resolved

Jackie-Jiang reviewed Feb 10, 2021

View reviewed changes

yupeng9 reviewed Feb 10, 2021

View reviewed changes

laxmanchekka mentioned this pull request Feb 17, 2021

Support replacing real-time committed segment #6362

Closed

snleee mentioned this pull request Mar 6, 2021

add uploadLLCSegment endpoint in TableResource #6653

Merged

xiangfu0 reviewed Mar 13, 2021

View reviewed changes

chenboat force-pushed the upsert-long-retention branch from 0b6a400 to 0f93df0 Compare March 15, 2021 06:18

mcvsubbu reviewed Mar 16, 2021

View reviewed changes

yupeng9 reviewed Mar 18, 2021

View reviewed changes

chenboat requested review from Jackie-Jiang, mcvsubbu, xiangfu0 and yupeng9 March 19, 2021 06:41

yupeng9 approved these changes Mar 20, 2021

View reviewed changes

pinot-common/src/main/java/org/apache/pinot/common/utils/fetcher/SegmentFetcherFactory.java Outdated Show resolved Hide resolved

pinot-common/src/main/java/org/apache/pinot/common/utils/fetcher/SegmentFetcherFactory.java Outdated Show resolved Hide resolved

Jackie-Jiang requested changes Mar 22, 2021

View reviewed changes

chenboat changed the title ~~Enable segment upload to a realtime table~~ Enable adding new segments. to a upsert-enabled realtime table Mar 23, 2021

Jackie-Jiang approved these changes Mar 23, 2021

View reviewed changes

chenboat changed the title ~~Enable adding new segments. to a upsert-enabled realtime table~~ Enable adding new segments to a upsert-enabled realtime table Mar 24, 2021

chenboat and others added 3 commits March 30, 2021 15:39

Allow upload segments to Pinot realtime tables.

fd8f45e

Fix comments and add more unit test cases.

6c26468

Revised PR based on comments.

65be47a

chenboat and others added 11 commits March 30, 2021 15:43

Fix broken unit tests

79a490e

Fix the broken integration test.

dc6c580

Address review comments.

b5cf742

Address review comments.

bbfd50c

Address reviewer comments.

4ee8e7e

Address Jackie's comments

eeedde5

Minor style fix.

1f055af

Add realtime data ingestion to the integration tests with 5 segments …

bfe9ba1

…loaded: 2 realtime and 3 offline.

Address Jackie's comments.

b104206

Add tests for realtime validation manager.

132dd70

Add an integration test resource.

f987ce8

chenboat force-pushed the upsert-long-retention branch from 38cca2c to f987ce8 Compare March 31, 2021 17:16

chenboat added 2 commits March 31, 2021 10:24

Delete wrong commit files.

5d7663b

Fix an integration test which uses tablename w/o type to download seg…

fa265a3

…ments.

mcvsubbu reviewed Apr 2, 2021

View reviewed changes

chenboat and others added 4 commits April 2, 2021 15:41

Update pinot-common/src/main/java/org/apache/pinot/common/utils/Commo…

e2a94d9

…nConstants.java Co-authored-by: Subbu Subramaniam <mcvsubbu@users.noreply.github.com>

Update pinot-common/src/main/java/org/apache/pinot/common/utils/Commo…

d1b4396

…nConstants.java Co-authored-by: Subbu Subramaniam <mcvsubbu@users.noreply.github.com>

Fix based on Subbu's comments

96f5257

Remove two enums.

1452dad

chenboat requested a review from mcvsubbu April 5, 2021 18:44

Merge branch 'master' into upsert-long-retention

c74fd56

mcvsubbu reviewed Apr 12, 2021

View reviewed changes

Move the log line for segment refreshing case.

e27f04e

mcvsubbu approved these changes Apr 12, 2021

View reviewed changes

chenboat merged commit 310c44e into apache:master Apr 12, 2021

chenboat deleted the upsert-long-retention branch April 12, 2021 20:53

Jackie-Jiang mentioned this pull request Mar 2, 2022

Support segment upload to real-time table #8283

Closed

rohityadav1993 mentioned this pull request Apr 22, 2024

Upsert table backfill enhancement: support externally partitioned data #12987

Open

Enable adding new segments to a upsert-enabled realtime table #6567

Enable adding new segments to a upsert-enabled realtime table #6567

Conversation

chenboat commented Feb 9, 2021

Description

Upgrade Notes

Release Notes

Documentation

codecov-io commented Feb 10, 2021 • edited Loading

Codecov Report

mcvsubbu left a comment

Choose a reason for hiding this comment

Jackie-Jiang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chenboat Mar 16, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

snleee commented Feb 10, 2021

chenboat commented Feb 10, 2021

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

yupeng9 left a comment

Choose a reason for hiding this comment

Jackie-Jiang left a comment

Choose a reason for hiding this comment

chenboat commented Mar 22, 2021

Jackie-Jiang commented Mar 23, 2021

chenboat commented Mar 23, 2021

Jackie-Jiang left a comment

Choose a reason for hiding this comment

Jackie-Jiang commented Mar 24, 2021

chenboat commented Mar 26, 2021 • 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

jtao15 commented Jun 15, 2021

nisheet195 commented Sep 2, 2021 • edited Loading

codecov-io commented Feb 10, 2021 •

edited

Loading

chenboat Mar 16, 2021 •

edited

Loading

chenboat commented Mar 26, 2021 •

edited

Loading

nisheet195 commented Sep 2, 2021 •

edited

Loading