Skip to content

apollo assembly optimization #5035

Merged
nobodyiam merged 70 commits intoapolloconfig:masterfrom
vdiskg:assembly
Feb 3, 2024
Merged

apollo assembly optimization #5035
nobodyiam merged 70 commits intoapolloconfig:masterfrom
vdiskg:assembly

Conversation

@vdiskg
Copy link
Copy Markdown
Contributor

@vdiskg vdiskg commented Dec 6, 2023

What's the purpose of this PR

  1. Assembly Portal, Config Service, Admin Service into a standalone process
  2. Assembly ApolloConfigDB, ApolloPortalDB into a single db.

Which issue(s) this PR fixes:

#4313
#4729

Brief changelog

  1. add table name prefix in the assembly sql
  2. add TablePrefixNamingStrategy to match the table name prefix

Follow this checklist to help us incorporate your contribution quickly and easily:

  • Read the Contributing Guide before making this pull request.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Write necessary unit tests to verify the code.
  • Run mvn clean test to make sure this pull request doesn't break anything.
  • Update the CHANGES log.

@codecov
Copy link
Copy Markdown

codecov bot commented Dec 6, 2023 •
edited
Loading

Codecov Report

Attention: 240 lines in your changes are missing coverage. Please review.

Comparison is base (921af84) 49.65% compared to head (e8b44ee) 50.38%.

Files Patch % Lines
...lloDataSourceScriptDatabaseInitializerFactory.java 0.00% 75 Missing ⚠️
...lo/build/sql/converter/ApolloSqlConverterUtil.java 77.72% 35 Missing and 12 partials ⚠️
.../datasource/ApolloSqlInitializationProperties.java 0.00% 32 Missing ⚠️
...rce/ApolloDataSourceScriptDatabaseInitializer.java 0.00% 25 Missing ⚠️
...ework/apollo/build/sql/converter/SqlStatement.java 55.55% 10 Missing and 2 partials ⚠️
...llo/build/sql/converter/ApolloH2ConverterUtil.java 91.52% 6 Missing and 4 partials ⚠️
...apollo/build/sql/converter/ApolloSqlConverter.java 86.36% 9 Missing ⚠️
...apollo/build/sql/converter/SqlTemplateContext.java 62.50% 6 Missing ⚠️
...rk/apollo/build/sql/converter/SqlTemplateGist.java 82.35% 6 Missing ⚠️
...sql/converter/ApolloMysqlDefaultConverterUtil.java 78.26% 4 Missing and 1 partial ⚠️
... and 7 more
Additional details and impacted files 
@@             Coverage Diff              @@
##             master    #5035      +/-   ##
============================================
+ Coverage     49.65%   50.38%   +0.72%     
- Complexity     1908     2012     +104     
============================================
  Files           372      387      +15     
  Lines         11564    12213     +649     
  Branches       1127     1207      +80     
============================================
+ Hits           5742     6153     +411     
- Misses         5479     5697     +218     
- Partials        343      363      +20

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@nobodyiam
Copy link
Copy Markdown
Member

nobodyiam commented Dec 7, 2023

Wow! This pr is truly thrilling!

@vdiskg vdiskg marked this pull request as ready for review December 14, 2023 14:05
@dosubot dosubot bot added the size:XXL This PR changes 1000+ lines, ignoring generated files. label Dec 14, 2023
@vdiskg vdiskg requested a review from nobodyiam December 14, 2023 14:06
nobodyiam
nobodyiam reviewed Dec 18, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@nobodyiam nobodyiam left a comment

Choose a reason for hiding this comment

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

Now the whole apollo servers could be launched in one process, this is amazing!
Would you please also update the apollo-development-guide.md?

image

Comment thread ...ice/src/main/java/com/ctrip/framework/apollo/adminservice/AdminServiceAutoConfiguration.java Outdated
Comment thread apollo-assembly/src/main/resources/application-github.properties Outdated
Comment thread docs/en/deployment/quick-start.md
No configuration is required, just use the following command to start
> Note: When using the in-memory database, any operation will be lost after the Apollo process restarts
```bash
export SPRING_PROFILES_ACTIVE="github,auth"
Copy link
Copy Markdown
Member

@nobodyiam nobodyiam Dec 18, 2023

Choose a reason for hiding this comment

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

Is it feasible to enhance the demo.sh script to enable users to initiate the quick-start demonstration efficiently through some configuration settings?

@nobodyiam
Copy link
Copy Markdown
Member

nobodyiam commented Dec 18, 2023 •
edited
Loading

It appears there is some incompatibility with database-discovery. When I execute the assembly using the spring profiles github,auth,database-discovery, only the apollo-configservice gets registered.

image

github-advanced-security[bot]
github-advanced-security AI found potential problems Jan 12, 2024
View reviewed changes
Comment thread ...src/main/java/com/ctrip/framework/apollo/adminservice/AdminServiceAssemblyConfiguration.java

@Override
protected void configure(HttpSecurity http) throws Exception {
http.csrf().disable();

Check failure

Code scanning / CodeQL

Disabled Spring CSRF protection

CSRF vulnerability due to protection being disabled.
@nobodyiam
Copy link
Copy Markdown
Member

nobodyiam commented Jan 13, 2024

This pull request appears to be in good shape now. Could you also update the CHANGES.md file? Since this constitutes a major change, it's important to document it there.

@nobodyiam
Copy link
Copy Markdown
Member

nobodyiam commented Jan 13, 2024

BTW, #5072 was merged with a schema change, would you please help to update the code base again?

nobodyiam
nobodyiam reviewed Jan 27, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@nobodyiam nobodyiam left a comment

Choose a reason for hiding this comment

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

Please also fix the failed tests.

2024-01-26T16:03:31.8898714Z Caused by: java.lang.IllegalStateException: @order on WebSecurityConfigurers must be unique. Order of 99 was already used on com.ctrip.framework.apollo.adminservice.controller.TestWebSecurityConfig$$EnhancerBySpringCGLIB$$ac5b1e3@469959b6, so it cannot be used on com.ctrip.framework.apollo.adminservice.AdminServiceAssemblyConfiguration$AdminServiceSecurityConfigurer$$EnhancerBySpringCGLIB$$8154145d@270147e3 too.

Comment thread apollo-assembly/src/main/resources/logback.xml Outdated
vdiskg added 6 commits February 2, 2024 19:49
@vdiskg
clean logger
4d72047
@vdiskg
Merge branch 'master' into assembly
a8398e5 
# Conflicts:
#	docs/en/contribution/apollo-development-guide.md
#	docs/zh/contribution/apollo-development-guide.md
@vdiskg
fix order
4941f4f
@vdiskg
fix h2 converter
06afabb
@vdiskg
fix h2 converter v0.2
d68ed97
@vdiskg
fix order v0.2
e8b44ee
nobodyiam
nobodyiam approved these changes Feb 3, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@nobodyiam nobodyiam left a comment

Choose a reason for hiding this comment

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

LGTM

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Feb 3, 2024
@nobodyiam nobodyiam merged commit c90c930 into apolloconfig:master Feb 3, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Feb 3, 2024
@nobodyiam nobodyiam added this to the 2.3.0 milestone Apr 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@nobodyiam nobodyiam nobodyiam approved these changes

Assignees

No one assigned

Labels

lgtm This PR has been approved by a maintainer size:XXL This PR changes 1000+ lines, ignoring generated files.

Projects

None yet

Milestone

2.3.0

Development

Successfully merging this pull request may close these issues.

3 participants

@vdiskg @nobodyiam @github-advanced-security