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

Support prepare and save metrics concurrency #7153

Merged
merged 47 commits into from
Jun 29, 2021

Conversation

sN0wpeak
Copy link
Contributor

@sN0wpeak sN0wpeak commented Jun 22, 2021

@codecov
Copy link

codecov bot commented Jun 22, 2021

Codecov Report

Merging #7153 (1d9be65) into master (de1d046) will increase coverage by 4.08%.
The diff coverage is 76.31%.

❗ Current head 1d9be65 differs from pull request most recent head e453239. Consider uploading reports for the commit e453239 to get more accurate results
Impacted file tree graph

@@             Coverage Diff              @@
##             master    #7153      +/-   ##
============================================
+ Coverage     53.56%   57.65%   +4.08%     
+ Complexity     4294     4275      -19     
============================================
  Files          1887     1021     -866     
  Lines         40709    26243   -14466     
  Branches       4572     2605    -1967     
============================================
- Hits          21807    15130    -6677     
+ Misses        17827     9776    -8051     
- Partials       1075     1337     +262     
Impacted Files Coverage Δ
...king/oap/server/core/storage/PersistenceTimer.java 72.17% <75.67%> (+3.60%) ⬆️
...e/skywalking/oap/server/core/CoreModuleConfig.java 92.50% <100.00%> (-7.50%) ⬇️
...g/oap/server/core/cluster/ClusterHealthStatus.java 0.00% <0.00%> (-100.00%) ⬇️
...p/server/core/analysis/metrics/MinLongMetrics.java 0.00% <0.00%> (-100.00%) ⬇️
...er/exporter/provider/grpc/GRPCExporterSetting.java 0.00% <0.00%> (-100.00%) ⬇️
...king/oap/server/configuration/api/ConfigTable.java 0.00% <0.00%> (-100.00%) ⬇️
...skywalking/oap/server/receiver/envoy/als/Role.java 0.00% <0.00%> (-100.00%) ⬇️
...t/status/HierarchyMatchExceptionCheckStrategy.java 9.09% <0.00%> (-90.91%) ⬇️
...csearch/query/ProfileThreadSnapshotQueryEsDAO.java 4.21% <0.00%> (-90.53%) ⬇️
...ywalking/oap/server/library/util/ConnectUtils.java 0.00% <0.00%> (-90.00%) ⬇️
... and 1329 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 de1d046...e453239. Read the comment docs.

@wu-sheng
Copy link
Member

You should update the changelog.

@wu-sheng wu-sheng added this to the 8.7.0 milestone Jun 22, 2021
@wu-sheng wu-sheng added backend OAP backend related. enhancement Enhancement on performance or codes labels Jun 22, 2021
@wu-sheng
Copy link
Member

And CIs fail, please recheck.

…/server/core/storage/PersistenceTimer.java

Co-authored-by: kezhenxu94 <kezhenxu94@apache.org>
@wu-sheng wu-sheng requested a review from kezhenxu94 June 26, 2021 15:28
Copy link
Member

@wu-sheng wu-sheng left a comment

Choose a reason for hiding this comment

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

LGTM. @dmsolr @EvanLjp Please check codes, more feedback is welcome.

Copy link
Member

@EvanLjp EvanLjp left a comment

Choose a reason for hiding this comment

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

LGTM, It's more clear than before.

@wu-sheng
Copy link
Member

@a1vin-tian Before merge, could you run this latest codes on prod or test env, let's see how self observability looks like.

@sN0wpeak
Copy link
Contributor Author

@a1vin-tian Before merge, could you run this latest codes on prod or test env, let's see how self observability looks like.

Okay, I will do it these two days.

@sN0wpeak
Copy link
Contributor Author

@wu-sheng

image

After:
image

Befor:
image
as time goes on.
image

@wu-sheng
Copy link
Member

@a1vin-tian Thank you so much for your good polish, patient, and keeping testing in real env. I am going to merge this.

@wu-sheng wu-sheng merged commit a966eea into apache:master Jun 29, 2021
wu-sheng added a commit that referenced this pull request Jun 29, 2021
…alking into id-read-optimization

* 'id-read-optimization' of https://github.com/apache/skywalking:
  Support prepare and save metrics concurrency (#7153)
  Update MetricsPersistentWorker.java

# Conflicts:
#	oap-server/server-core/src/main/java/org/apache/skywalking/oap/server/core/analysis/worker/MetricsPersistentWorker.java
wu-sheng added a commit that referenced this pull request Jul 16, 2021
wu-sheng added a commit that referenced this pull request Jul 17, 2021
… and concurrency process (#7318)

 The key logic behinds all these is, metrics persistence is fully asynchronous.

* The core/maxSyncOperationNum setting(added in 8.5.0) is removed due to metrics persistence is fully asynchronous.
* The core/syncThreads setting(added in 8.5.0) is removed due to metrics persistence is fully asynchronous.
* Optimization: Concurrency mode of execution stage for metrics is removed(added in 8.5.0). The only concurrency of prepare stage is meaningful and kept.
* Remove the outside preparedRequest list initialization, worker instance could always build a suitable size list in the first place (Reduce Array.copy and GC load a little).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend OAP backend related. enhancement Enhancement on performance or codes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Concurrent create PrepareRequest when persist Metrics
5 participants