Refactor SpiLoader and enhance its SPI functions #1383

cdfive · 2020-04-05T14:47:20Z

Describe what this PR does / why we need it

Refer and compared with some SPI implementations, ServiceLoader of JDK, ExtensionLoader of Dubbo and SOFARPC, I'd like to introduce some of their advantages and try to enhance SpiLoader of Sentinel, to provide more functions to support more scenarios, such as singleton/multiple instance, loaded by alias name, default Provider of SPI, etc.

Does this pull request fix one issue?

Fixes #1379

Describe how you did it

Add @Spi annotation with following fields:

value
aliasname of concrete Provider
by default is Provider class name
isSingleton
whether create singleton Provider instance
by default is true, since singleton is more common
isDefault
whether is the default Provider
by default is false, if it's the default implementation, we explicitly identify it
order
order priority of concrete Provider class
by default is 0

For Provider class which need create multiple instances, such as NodeSelectorSlot:

@Spi(isSingleton = false, order = Constants.ORDER_NODE_SELECTOR_SLOT)
public class NodeSelectorSlot

For Provider class which create singleton instance, such as FlowSlot:

@Spi(order = Constants.ORDER_FLOW_SLOT)
public class FlowSlot

For Provider class which is the default Provider, such as DefaultSlotChainBuilder:

@Spi(isDefault = true)
public class DefaultSlotChainBuilder

I didn't exactly copy the way Dubbo and SOFARPC did, and maybe thought as an upgrade of default JDK's SPI, that isServiceLoader, and enhance its functions.
Along this line of thought, the usage is exactly same as JDK's SPI, we can also omit the @Spi annotation if wanted, worked with default behavior.

Functions of SpiLoader:

Load Provider instance list
loadInstanceList/loadInstanceListSorted
Load specific order or default Provider instance
loadHighestPriorityInstance/loadLowestPriorityInstance/loadDefaultInstance
Load one specific Provider via Class or aliasname
loadInstance(Class)/loadInstance(String aliasName)

Since the order of ProcessorSlot is very important, I add constant variables in Constants class for the order value of @Spi, so we can see the order of system default slots more intuitively. The GatewayFlowSlot and ParamFlowSlot are in other module, so add with comment lines.

Constants:

/**
 * Order of default processor slots
 */
public static final int ORDER_NODE_SELECTOR_SLOT = -10000;
public static final int ORDER_CLUSTER_BUILDER_SLOT = -9000;
public static final int ORDER_LOG_SLOT = -8000;
public static final int ORDER_STATISTIC_SLOT = -7000;
public static final int ORDER_AUTHORITY_SLOT = -6000;
public static final int ORDER_SYSTEM_SLOT = -5000;
// order in GatewayFlowSlot -4000
// order in ParamFlowSlot   -3000
public static final int ORDER_FLOW_SLOT = -2000;
public static final int ORDER_DEGRADE_SLOT = -1000;

SpiLoader has been moved to spi package, the same as @Spi.
@SpiOrder,ServiceLoaderUtil has been removed since SpiLoader include the functions.
Not add more class like ExtensionClass in SOFARPC, ExtentionLoaderFactory in Dubbo, just one SpiLoder class, to reduce complexity and make it simple.

Add SpiLoaderException class for thrown when something goes wrong while loading Provider.

Describe how to verify it

Run test cases.

Special notes for reviews

At first I use singleton as field name, when adding default field, found that it's keyword of Java. So modified with isSingleton,isDefault, the is prefix hinted that it's a boolean value with true/false. Maybe there are other better ways to name them, please tell me and I will optimize it.

Now the implementation may not very perfect, please take a review and I'd like to improve the code step by step.

sczyh30 · 2020-04-27T07:07:02Z

Could you please resolve conflicts?

…nstanceOrDefault method, improve test cases

cdfive · 2020-04-28T15:45:54Z

Could you please resolve conflicts?

Rebase the branch and the conflicts have been resolved.

CLAassistant · 2020-06-12T07:21:40Z

All committers have signed the CLA.

jasonjoo2010 · 2020-08-05T03:16:53Z

sentinel-core/src/test/resources/META-INF/services/com.alibaba.csp.sentinel.spi.TestInterface

@@ -0,0 +1,5 @@
+# One
+com.alibaba.csp.sentinel.spi.TestOneProvider
+com.alibaba.csp.sentinel.spi.TestTwoProvider # Two


Suggest to beautify the comment. Move it to a separated line maybe.

Yes, normally we use a separated line to write the comment, which is more beautify and clear.
Here is intended to write the comment like this, to test the robustness of the program in test case.

jasonjoo2010 · 2020-08-05T03:26:58Z

...mmon/src/main/java/com/alibaba/csp/sentinel/adapter/gateway/common/slot/GatewayFlowSlot.java


 /**
 * @author Eric Zhao
 * @since 1.6.1
 */
-@SpiOrder(-4000)
+@Spi(order = -4000)


Use constant?

And some slots are stateless while others are stateful. Maybe isSingleton should be specified obviously.

I'm a little confused here, not very sure and use -4000 temporarily.
There are some constants of slot order definitions In Constants class of sentinel-core module:

/** * Order of default processor slots */ public static final int ORDER_NODE_SELECTOR_SLOT = -10000; public static final int ORDER_CLUSTER_BUILDER_SLOT = -9000; public static final int ORDER_LOG_SLOT = -8000; public static final int ORDER_STATISTIC_SLOT = -7000; public static final int ORDER_AUTHORITY_SLOT = -6000; public static final int ORDER_SYSTEM_SLOT = -5000; // order of GatewayFlowSlot -4000 // order of ParamFlowSlot -3000 public static final int ORDER_FLOW_SLOT = -2000; public static final int ORDER_DEGRADE_SLOT = -1000;

If use constant, which is better?

public static final int ORDER_GATEWAY_FLOW_SLOT = -4000; public static final int ORDER_PARAM_FLOW_SLOT = -3000; @Spi(order = Constants.ORDER_SYSTEM_SLOT) public class GatewayFlowSlot {

No constant for ORDER_GATEWAY_FLOW_SLOT, ORDER_PARAM_FLOW_SLOT @Spi(order = Constants.ORDER_SYSTEM_SLOT + 1000) public class GatewayFlowSlot {

Constants.ORDER_SYSTEM_SLOT

Yeah whatever you want.

If you want to make it reference-able just replace it with a constant in gateway package;
If you want to express some kinds of relationship with existing slots just use relative calculation to existed constants;
If you insist on pure definition just leave it unchanged.

These three ways speak my heart. I've been struggling with this for a long time.
Now I think the third way is not very good. Maybe one or two is better.
For a selection difficulty person, could you please help to make a good choice?

These three ways speak my heart. I've been struggling with this for a long time.
Now I think the third way is not very good. Maybe one or two is better.
For a selection difficulty person, could you please help to make a good choice?

Go write a random test and make result confirmed.

Got it! Using the first way, since it expresses the order of all default slot of sentinel clearly. Maybe the second way is a good choice for user custom slot.

sentinel-core/src/main/java/com/alibaba/csp/sentinel/spi/SpiLoader.java

jasonjoo2010 · 2020-08-05T03:51:08Z

sentinel-core/src/main/java/com/alibaba/csp/sentinel/spi/SpiLoader.java

+     * @param <T>     Service type
+     * @return SpiLoader instance
+     */
+    public static <T> SpiLoader<T> getSpiLoader(Class<T> service) {


private or package default scope is suggested.

This method is intend to provide another method to create SpiLoader instance, and inside it just call of method.
How about remove it? Maybe of method is enough.
Just now, I found that in JDK's Optional, apache commons's Pair, they only have of method to create its instance.

Yeah i think this scaffold class should be shorter so just don't be limited by legacy design.

This unused method has been removed, only of method is remained.

jasonjoo2010 · 2020-08-05T03:56:39Z

sentinel-core/src/main/java/com/alibaba/csp/sentinel/spi/SpiLoader.java

+     * Reset all SpiLoader instances
+     */
+    public static void resetAll() {
+        Set<Map.Entry<String, SpiLoader>> entries = SPI_LOADER_MAP.entrySet();


Maybe we should also sychronize it?

And it looks more like resetAndClear()

Are reset*() only used in unit tests?
If so i suggest to comment them obviously and make them package scope is better.

They have been improved, please check.

jasonjoo2010 · 2020-08-05T04:26:55Z

sentinel-core/src/main/java/com/alibaba/csp/sentinel/spi/SpiLoader.java

+     * @param closeables {@link Closeable} resources
+     */
+    private void closeResources(Closeable... closeables) {
+        if (closeables == null) {


Maybe len=0 should also be added here for more obvious.

Good idea, it has been added.

It has been improved.

jasonjoo2010 · 2020-08-05T04:27:26Z

sentinel-core/src/main/java/com/alibaba/csp/sentinel/spi/SpiLoader.java

+            try {
+                closeable.close();
+            } catch (Exception e) {
+                fail("error closing SPI file", e);


SPI -> resource ?

Oh, SPI file may be confusing.
May I use error closing SPI resource file or error closing SPI configuration file?
This method is used for close InputStream and BufferedReader after reading and parsing configuration file.
SPI is used to emphasize the operation.

Oh, SPI file may be confusing.
May I use error closing SPI resource file or error closing SPI configuration file?
This method is used for close InputStream and BufferedReader after reading and parsing configuration file.
SPI is used to emphasize the operation.

File is already a kind of resource. Or do you mean you implemented a method which is more like closeFiles(...)?

The prompt message changed to error closing SPI configuration file, and the exception in closeResources method seems rarely happen.

sentinel-core/src/main/java/com/alibaba/csp/sentinel/spi/SpiLoader.java

jasonjoo2010 · 2020-08-05T04:32:10Z

sentinel-core/src/main/java/com/alibaba/csp/sentinel/spi/SpiLoader.java

+     *
+     * @return Provider instance of highest order priority
+     */
+    public S loadHighestPriorityInstance() {


Maybe loadOne for short. Is there any other way to get target instance? And surely i am curious that why we need loadLowestPriorityInstance or loadFirstInstance. And talk to isDefault in SPI annotation we can make the default service implementation as lowest priority. Then we can use loadOne() instead of something like loadFirstInstanceOrDefault.
Thus loadInstance loadDefault also don't have any scenario to be applied.

The loadHighestPriorityInstance,loadFirstInstance keeps same with previous version.
In SlotChainProvider, we have DefaultSlotChainBuilder has default implementation.

slotChainBuilder = SpiLoader.of(SlotChainBuilder.class).loadFirstInstanceOrDefault();

In TokenClientProvider, the difference is no default SPI implementation.

`ClusterTokenClient resolvedClient = SpiLoader.of(ClusterTokenClient.class).loadFirstInstance();`

Distinguish them may be more flexible.

jasonjoo2010 · 2020-08-05T04:43:31Z

...l-core/src/main/resources/META-INF/services/com.alibaba.csp.sentinel.slotchain.ProcessorSlot

 com.alibaba.csp.sentinel.slots.block.authority.AuthoritySlot
+com.alibaba.csp.sentinel.slots.system.SystemSlot


No need to change.

Fixed, now not changed in this PR.

…rove the log info,remove unused code in SpiLoader

jasonjoo2010 · 2020-08-09T16:17:09Z

sentinel-core/src/main/java/com/alibaba/csp/sentinel/Constants.java

+    public static final int ORDER_STATISTIC_SLOT = -7000;
+    public static final int ORDER_AUTHORITY_SLOT = -6000;
+    public static final int ORDER_SYSTEM_SLOT = -5000;
+    public static final int ORDER_GATEWAY_FLOW_SLOT = -4000;


It's not a good practice setting unrelated constant here. You'd better put it in gateway adapter package. If you feel a little unsure to decide the value of it in separated package you can take it as a user-defined implementation out of sentinel-core.

The two constant ORDER_GATEWAY_FLOW_SLOT,ORDER_PARAM_FLOW_SLOT have been removed here.

jasonjoo2010 · 2020-09-14T07:09:23Z

Hi, @cdfive

What's the plan on this PR?
Is there any available schedule?

cdfive · 2020-09-14T13:36:51Z

@jasonjoo2010 Sorry for late... I'm busy with my work now, maybe I can improve it this weekend.

jasonjoo2010 · 2020-09-14T15:15:01Z

@jasonjoo2010 Sorry for late... I'm busy with my work now, maybe I can improve it this weekend.

That's all right💪

…ment # Conflicts: # sentinel-core/src/main/java/com/alibaba/csp/sentinel/init/InitExecutor.java # sentinel-core/src/main/java/com/alibaba/csp/sentinel/metric/extension/MetricExtensionProvider.java # sentinel-core/src/main/java/com/alibaba/csp/sentinel/slotchain/SlotChainProvider.java # sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/degrade/DegradeSlot.java

cdfive · 2020-09-20T09:25:09Z

Conflicts have been resolved, but the integration-test run failed, I tried several times and have not found the point.

[INFO] sentinel-demo-quarkus .............................. FAILURE [ 11.949 s]

Could you please give me some help? @sczyh30 @jasonjoo2010

sczyh30 · 2020-09-20T14:15:54Z

Conflicts have been resolved, but the integration-test run failed, I tried several times and have not found the point.
[INFO] sentinel-demo-quarkus .............................. FAILURE [ 11.949 s]
Could you please give me some help? @sczyh30 @jasonjoo2010

Seems like an infinite slot chain was constructed?

2020-09-20 08:45:06,227 WARN  [sentinelRecordLogger] (executor-thread-1) Unexpected entry exception: java.lang.StackOverflowError
	at com.alibaba.csp.sentinel.slots.block.degrade.DegradeSlot.entry(DegradeSlot.java:43)
	at com.alibaba.csp.sentinel.slots.block.degrade.DegradeSlot.entry(DegradeSlot.java:37)
	at com.alibaba.csp.sentinel.slotchain.AbstractLinkedProcessorSlot.transformEntry(AbstractLinkedProcessorSlot.java:40)
	at com.alibaba.csp.sentinel.slotchain.AbstractLinkedProcessorSlot.fireEntry(AbstractLinkedProcessorSlot.java:32)
	at com.alibaba.csp.sentinel.slots.block.degrade.DegradeSlot.entry(DegradeSlot.java:45)
	at com.alibaba.csp.sentinel.slots.block.degrade.DegradeSlot.entry(DegradeSlot.java:37)
	at com.alibaba.csp.sentinel.slotchain.AbstractLinkedProcessorSlot.transformEntry(AbstractLinkedProcessorSlot.java:40)
	at com.alibaba.csp.sentinel.slotchain.AbstractLinkedProcessorSlot.fireEntry(AbstractLinkedProcessorSlot.java:32)
	at com.alibaba.csp.sentinel.slots.block.degrade.DegradeSlot.entry(DegradeSlot.java:45)
	at com.alibaba.csp.sentinel.slots.block.degrade.DegradeSlot.entry(DegradeSlot.java:37)
	at com.alibaba.csp.sentinel.slotchain.AbstractLinkedProcessorSlot.transformEntry(AbstractLinkedProcessorSlot.java:40)
	at com.alibaba.csp.sentinel.slotchain.AbstractLinkedProcessorSlot.fireEntry(AbstractLinkedProcessorSlot.java:32)
	at com.alibaba.csp.sentinel.slots.block.degrade.DegradeSlot.entry(DegradeSlot.java:45)
	at com.alibaba.csp.sentinel.slots.block.degrade.DegradeSlot.entry(DegradeSlot.java:37)
	at com.alibaba.csp.sentinel.slotchain.AbstractLinkedProcessorSlot.transformEntry(AbstractLinkedProcessorSlot.java:40)
	at com.alibaba.csp.sentinel.slotchain.AbstractLinkedProcessorSlot.fireEntry(AbstractLinkedProcessorSlot.java:32)

sentinel-core/src/main/java/com/alibaba/csp/sentinel/spi/SpiLoader.java

…ception occurs.

…nning test cases.

cdfive · 2020-10-02T05:13:18Z

2020-09-20 08:45:06,227 WARN [sentinelRecordLogger] (executor-thread-1) Unexpected entry exception: java.lang.StackOverflowError
at com.alibaba.csp.sentinel.slots.block.degrade.DegradeSlot.entry(DegradeSlot.java:43)
at com.alibaba.csp.sentinel.slots.block.degrade.DegradeSlot.entry(DegradeSlot.java:37)

I located the exception, which is caused by more than one SPI file, and the sortedClassList added more than once in SpiLoader, it's a bug. Now it has been fixed.
PTAL @sczyh30 @jasonjoo2010

nickChenyx

cool

cdfive · 2020-10-08T06:35:00Z

@nickChenyx I'm sorry, the double check when creating new instance is incorrect indeed. It has been fixed(after synchronized (this), check instance not null again, it's really necessary since the instance is a local variable).
Thanks for help reviewing!

canglang1973 · 2020-11-07T08:06:42Z

This is a good enhancement! @cdfive

cdfive · 2020-11-07T15:13:26Z

@canglang1973 Thanks! Come together to review the code.

jasonjoo2010

This PR lasts long. There may be still some improvements on it which can be also done later as another PR. And there is indeed some possible performance issue in SpiLoader.
So shall we keep improvements in it or a new PR? I need your suggestions. @sczyh30 @cdfive

jasonjoo2010 · 2020-12-23T06:23:32Z

sentinel-core/src/main/java/com/alibaba/csp/sentinel/spi/SpiLoader.java

+     * @return Provider class
+     */
+    private S createInstance(Class<? extends S> clazz) {
+        Spi spi = clazz.getAnnotation(Spi.class);


Will it remain for future improvement? About meta caching.

jasonjoo2010 · 2020-12-23T06:26:26Z

sentinel-core/src/main/java/com/alibaba/csp/sentinel/spi/SpiLoader.java

+     * @param clazzList class types of Providers
+     * @return Provider instance list
+     */
+    private List<S> createInstanceList(List<Class<? extends S>> clazzList) {


And this one? Remain it this time?

cdfive · 2020-12-23T14:09:39Z

Will it remain for future improvement? About meta caching.

是指要像Spring的AnnotationUtils类一样对元数据做缓存吗？
项目里其它模块都是class. getAnnotation这样取的，SOFARpc里也是，这个可否由单独PR来优化

jasonjoo2010 · 2020-12-23T14:58:24Z

Will it remain for future improvement? About meta caching.

是指要像Spring的AnnotationUtils类一样对元数据做缓存吗？
项目里其它模块都是class. getAnnotation这样取的，SOFARpc里也是，这个可否由单独PR来优化

对类似，性能相关可以考虑分阶段改进
目前对于功能性正常的话可以考虑尽快合一下

sczyh30

LGTM

sczyh30 · 2021-01-26T10:41:55Z

Awesome work. Thanks for contributing!

As this PR contains breaking changes, we may need to describe this in the release note of the next version.

jasonjoo2010

LGTM

sczyh30 · 2021-01-27T01:46:46Z

👍 🥂

* Add `@Spi` annotation as the general annotation for SPI definition. * Add isDefault in @SPI, add loadDefaultInstance and improve loadFirstInstanceOrDefault method, improve test cases * Add SpiLoaderException class for thrown when something goes wrong while loading Provider * Rearrange packages of base SPI mechanism NOTE: this PR contains breaking changes regarding API.

wuwen5 · 2021-05-28T00:59:29Z

建议小版本号的更新保持下兼容性，因为与其他子模块之间不兼容。用户仅仅只是将sentinel-core升级一位小版本,从1.8.0升级到1.8.1 导致服务不可用，与sentinel-transport-common,sentinel-cluster-client-default,sentinel-cluster-server-default子模块之间不兼容.

* Add `@Spi` annotation as the general annotation for SPI definition. * Add isDefault in @SPI, add loadDefaultInstance and improve loadFirstInstanceOrDefault method, improve test cases * Add SpiLoaderException class for thrown when something goes wrong while loading Provider * Rearrange packages of base SPI mechanism NOTE: this PR contains breaking changes regarding API.

Refactor SpiLoader and enhance SPI mechanism (alibaba#1383)

* Add `@Spi` annotation as the general annotation for SPI definition. * Add isDefault in @SPI, add loadDefaultInstance and improve loadFirstInstanceOrDefault method, improve test cases * Add SpiLoaderException class for thrown when something goes wrong while loading Provider * Rearrange packages of base SPI mechanism NOTE: this PR contains breaking changes regarding API.

cdfive requested review from sczyh30 and jasonjoo2010 April 5, 2020 15:27

sczyh30 added to-review To review kind/enhancement Category issues or prs related to enhancement. size/XXL Indicate a PR that changes 1000+ lines. labels Apr 6, 2020

cdfive added 4 commits April 28, 2020 23:24

Refactor SpiLoader and enhance its SPI functions.

47953f0

Add isDefault in @SPI, add loadDefaultInstance and improve loadFirstI…

800eb76

…nstanceOrDefault method, improve test cases

Fix no SPI file scene and test cases

d5cc3e7

Improve init and fix comment

d3926c4

cdfive force-pushed the spiloader_enhancement branch from d958a5b to d3926c4 Compare April 28, 2020 15:27

sczyh30 added the kind/refactor Issue related to functional refactoring. label May 18, 2020

jasonjoo2010 self-assigned this Jul 22, 2020

jasonjoo2010 added this to the 1.8.1 milestone Jul 22, 2020

jasonjoo2010 requested changes Aug 5, 2020

View reviewed changes

cdfive added 2 commits August 9, 2020 14:38

Using constant for the order of GatewayFlowSlot and ParamFlowSlot,imp…

974bf09

…rove the log info,remove unused code in SpiLoader

Make reset method package privilege

95097bf

jasonjoo2010 reviewed Aug 9, 2020

View reviewed changes

cdfive added 3 commits September 20, 2020 15:27

Merge from master and improve code based on review

f3ed638

Fix merge compile error

da43f37

nickChenyx reviewed Sep 21, 2020

View reviewed changes

sentinel-core/src/main/java/com/alibaba/csp/sentinel/spi/SpiLoader.java Show resolved Hide resolved

sentinel-core/src/main/java/com/alibaba/csp/sentinel/spi/SpiLoader.java Show resolved Hide resolved

Fix double check when create instance,and fix close resources when ex…

e312d8a

…ception occurs.

Fix sortedClassList added more than once which cause CI error when ru…

ffa8d0c

…nning test cases.

nickChenyx reviewed Oct 8, 2020

View reviewed changes

Fix double check when create new instance in SpiLoader

d236171

jasonjoo2010 requested changes Dec 23, 2020

View reviewed changes

sczyh30 approved these changes Jan 26, 2021

View reviewed changes

sczyh30 added api-change Issues or PRs related to api changes and removed to-review To review labels Jan 26, 2021

jasonjoo2010 approved these changes Jan 26, 2021

View reviewed changes

sczyh30 merged commit 62efb78 into alibaba:master Jan 27, 2021

sczyh30 mentioned this pull request Jan 31, 2021

integration transport module with spring-mvc #1957

Merged

JasonMing mentioned this pull request May 7, 2021

SpiLoader was broken from 1.8.0 to 1.8.1 #2193

Closed

linkolen added a commit to shivagowda/Sentinel that referenced this pull request Aug 2, 2021

Merge pull request #3 from shivagowda/taha-master-patches-01

132d767

Refactor SpiLoader and enhance SPI mechanism (alibaba#1383)

ZShUn mentioned this pull request Apr 21, 2024

Spi重复初始化资源问题 #3376

Closed

		com.alibaba.csp.sentinel.slots.block.authority.AuthoritySlot
		com.alibaba.csp.sentinel.slots.system.SystemSlot

Refactor SpiLoader and enhance its SPI functions #1383

Refactor SpiLoader and enhance its SPI functions #1383

Conversation

cdfive commented Apr 5, 2020 • edited Loading

Describe what this PR does / why we need it

Does this pull request fix one issue?

Describe how you did it

Describe how to verify it

Special notes for reviews

sczyh30 commented Apr 27, 2020

cdfive commented Apr 28, 2020

CLAassistant commented Jun 12, 2020 • 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

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

cdfive Aug 5, 2020 • 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

jasonjoo2010 commented Sep 14, 2020

cdfive commented Sep 14, 2020

jasonjoo2010 commented Sep 14, 2020

cdfive commented Sep 20, 2020

sczyh30 commented Sep 20, 2020

cdfive commented Oct 2, 2020

nickChenyx left a comment

Choose a reason for hiding this comment

cdfive commented Oct 8, 2020 • edited Loading

canglang1973 commented Nov 7, 2020

cdfive commented Nov 7, 2020

jasonjoo2010 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cdfive commented Dec 23, 2020 • edited Loading

jasonjoo2010 commented Dec 23, 2020

sczyh30 left a comment

Choose a reason for hiding this comment

sczyh30 commented Jan 26, 2021

jasonjoo2010 left a comment

Choose a reason for hiding this comment

sczyh30 commented Jan 27, 2021

wuwen5 commented May 28, 2021

cdfive commented Apr 5, 2020 •

edited

Loading

CLAassistant commented Jun 12, 2020 •

edited

Loading

cdfive Aug 5, 2020 •

edited

Loading

cdfive commented Oct 8, 2020 •

edited

Loading

cdfive commented Dec 23, 2020 •

edited

Loading