Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

optimize: increase the cache of configuration values #2611

Merged
merged 72 commits into from
Jun 22, 2020

Conversation

funky-eyes
Copy link
Contributor

@funky-eyes funky-eyes commented Apr 24, 2020

Ⅰ. Describe what this PR did

optimize: increase the cache of configuration values

大并发下性能相对1.2提升可达90%+

SEATA 1.3(#2611) 性能测试报告

方式说明

nacos做配置&注册中心.

对40条数据随机forupdate读取后,update.设置最高并发200,请求数400,每种方式4次测试

Dubbo下测试: 暂无.

SpringCloud下测试: 678ms 431ms 500ms 427ms (全部正常响应)
File配置中心下测试: 358ms 553ms 421ms 387ms (全部正常响应)
本地事务下测试: 191ms 214ms 109ms 154ms (全部正常响应)

SEATA 1.2下测试: 10289ms 13931ms 11118ms 12792ms (70%+的请求接口响应全部超时)

测试环境

硬件环境: CPU: i5-8300
内存: 8G
硬盘:1TB SSD

软件环境:
系统:win10 64

JAVA: 1.8.0_181

Mysql: 5.7.23

Nacos: 1.2.1

测试模块

测试用例

测试工具

Ⅱ. Does this pull request fix one issue?

Ⅲ. Why don't you add test cases (unit test/integration test)?

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@funky-eyes funky-eyes closed this Apr 25, 2020
@funky-eyes funky-eyes reopened this Apr 25, 2020
@codecov-io
Copy link

codecov-io commented Apr 25, 2020

Codecov Report

Merging #2611 into develop will decrease coverage by 0.00%.
The diff coverage is 41.66%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #2611      +/-   ##
=============================================
- Coverage      50.82%   50.81%   -0.01%     
- Complexity      2815     2822       +7     
=============================================
  Files            558      559       +1     
  Lines          17931    17971      +40     
  Branches        2126     2132       +6     
=============================================
+ Hits            9113     9132      +19     
- Misses          7952     7968      +16     
- Partials         866      871       +5     
Impacted Files Coverage Δ Complexity Δ
...e/src/main/java/io/seata/config/Configuration.java 100.00% <ø> (ø) 1.00 <0.00> (ø)
...c/main/java/io/seata/config/ConfigurationKeys.java 0.00% <ø> (ø) 0.00 <0.00> (ø)
...a/io/seata/discovery/registry/RegistryService.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...ing/annotation/GlobalTransactionalInterceptor.java 8.94% <0.00%> (+0.14%) 2.00 <0.00> (ø)
...ain/java/io/seata/config/ConfigurationFactory.java 45.71% <30.76%> (-4.29%) 4.00 <0.00> (ø)
.../seata/config/SeataConfigurationCacheProvider.java 66.66% <66.66%> (ø) 8.00 <8.00> (?)
...in/java/io/seata/config/AbstractConfiguration.java 28.57% <100.00%> (+2.64%) 7.00 <1.00> (+1.00)
...c/main/java/io/seata/config/FileConfiguration.java 34.84% <100.00%> (ø) 6.00 <0.00> (ø)
...ta/spring/annotation/GlobalTransactionScanner.java 52.12% <100.00%> (ø) 17.00 <0.00> (ø)
...torage/file/store/FileTransactionStoreManager.java 56.77% <0.00%> (-0.65%) 28.00% <0.00%> (-1.00%)
... and 4 more

@funky-eyes funky-eyes closed this Apr 25, 2020
@funky-eyes funky-eyes reopened this Apr 25, 2020
@funky-eyes funky-eyes closed this Apr 25, 2020
@funky-eyes funky-eyes reopened this Apr 25, 2020
Copy link
Contributor

@objcoding objcoding left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I said my own understanding:

  1. The previous dynamic downgrade is to monitor the subscription through the configuration center and read the updated cache value as needed.
  2. you use caffeine as a cache locally, and then it expires regularly. There is no need to invalidate the cache regularly. The local cache uses monitoring subscriptions to update the cache in real time, and getConfig reads only the local cache.

The getConfig method will always only interact with the local cache. As for when the cache is updated, it will be handed over to the listen and subscribe logic instead of timing failure. If the local cache fails, it will still request the configuration center.

@funky-eyes funky-eyes closed this Apr 26, 2020
@funky-eyes funky-eyes reopened this Apr 26, 2020
@funky-eyes funky-eyes closed this Apr 26, 2020
@funky-eyes funky-eyes reopened this Apr 26, 2020
@slievrly slievrly self-requested a review April 26, 2020 09:12
@funky-eyes
Copy link
Contributor Author

I said my own understanding:

  1. The previous dynamic downgrade is to monitor the subscription through the configuration center and read the updated cache value as needed.
  2. you use caffeine as a cache locally, and then it expires regularly. There is no need to invalidate the cache regularly. The local cache uses monitoring subscriptions to update the cache in real time, and getConfig reads only the local cache.

The getConfig method will always only interact with the local cache. As for when the cache is updated, it will be handed over to the listen and subscribe logic instead of timing failure. If the local cache fails, it will still request the configuration center.

ok

@funky-eyes funky-eyes closed this Jun 6, 2020
@funky-eyes funky-eyes reopened this Jun 6, 2020
@funky-eyes funky-eyes closed this Jun 7, 2020
@funky-eyes funky-eyes reopened this Jun 7, 2020
Copy link
Contributor

@objcoding objcoding left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

about code style


private static final Set<String> LISTENER_KEYS = new HashSet<>();

private ConcurrentMap<String, HashSet<ConfigurationChangeListener>> configListenersMap =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just hashMap.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

if (CollectionUtils.isEmpty(listenerHashSet)) {
listenerHashSet = new HashSet<>();
}
for (int i = 0; i < listeners.length; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For code elegance may use foreach would be better.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

}
if (null != listeners && listeners.length > 0) {
HashSet<ConfigurationChangeListener> listenerHashSet = null;
try {
Copy link
Contributor

@objcoding objcoding Jun 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (null != listeners && listeners.length > 0) {
                HashSet<ConfigurationChangeListener> listenerHashSet =
                    getInstance().configListenersMap.computeIfAbsent(dataId, k -> new HashSet<>());
                if (CollectionUtils.isEmpty(listenerHashSet)) {
                    listenerHashSet = new HashSet<>();
                }
                for (ConfigurationChangeListener listener : listeners) {
                    ConfigurationChangeListener listener = listeners[i];
                    if (!listenerHashSet.contains(listener)) {
                        listenerHashSet.add(listener);
                        ConfigurationFactory.getInstance().addConfigListener(dataId, listener);
                    }
                }
            }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

Copy link
Contributor

@objcoding objcoding left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@funky-eyes
Copy link
Contributor Author

@slievrly PTAL

if (null != extConfiguration) {
configurationCache = ConfigurationCache.getInstance().proxy(configuration);
} else {
configurationCache = ConfigurationCache.getInstance().proxy(configuration);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check the code logic.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

degradeCheck = Boolean.parseBoolean(event.getNewValue());
if (!degradeCheck) {
if (ConfigurationKeys.CLIENT_DEGRADE_CHECK.equals(event.getDataId())) {
if (!Boolean.parseBoolean(event.getNewValue())) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why delete disable process logic?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the code has been restored

@@ -107,6 +136,10 @@ public Object invoke(final MethodInvocation methodInvocation) throws Throwable {
final GlobalTransactional globalTransactionalAnnotation =
getAnnotation(method, targetClass, GlobalTransactional.class);
final GlobalLock globalLockAnnotation = getAnnotation(method, targetClass, GlobalLock.class);
final boolean disable = ConfigurationFactory.getInstance()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why local variables?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if the disable configuration item does not exist?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if the disable configuration item does not exist?

when not present, the default value will be used

@funky-eyes funky-eyes requested a review from slievrly June 21, 2020 11:39
Copy link
Member

@slievrly slievrly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@jsbxyyx jsbxyyx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jsbxyyx jsbxyyx merged commit 73724a9 into apache:develop Jun 22, 2020
@yougecn
Copy link

yougecn commented Jul 10, 2020

期待1.3发布

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module/config config module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants