fail fast when protocol exp #12361
Merged
fail fast when protocol exp #12361
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
AlbumenJ
requested changes
May 21, 2023
...ubbo-registry-api/src/main/java/org/apache/dubbo/registry/integration/RegistryDirectory.java
Outdated
Show resolved
Hide resolved
AlbumenJ
reviewed
May 21, 2023
There was a problem hiding this comment.
Do we prepend to do this type check at initialization time. I think this modification is in the notify process. If dirty data is wrongly written in the registry (or an updated protocol, which usually occurs in the version iteration process), this will cause the processing of this push to be directly abnormal. , causing a greater impact.
Codecov Report
@@ Coverage Diff @@
## 3.2 #12361 +/- ##
============================================
- Coverage 69.64% 69.33% -0.32%
+ Complexity 341 2 -339
============================================
Files 3435 1622 -1813
Lines 161622 66857 -94765
Branches 27079 9802 -17277
============================================
- Hits 112566 46356 -66210
+ Misses 39169 16013 -23156
+ Partials 9887 4488 -5399 see 1936 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
The above is the thread stack of the current protocol verification, which is located in the process ReferenceConfig.init -> ReferenceConfig.createInvoker. Before the metadata is pushed, it seems that the registration center has not yet been pushed. Where do you want the protocol validation to be? |
Before listen registry. In |
ok |
wxbty
force-pushed
the
protocol_exp_bugfix
branch
from
408b5ec
to
ee5bd10
Compare
AlbumenJ
reviewed
May 29, 2023
There are protocols such as consumer that cannot be load |
org/apache/dubbo/registry/integration/RegistryDirectory.java:393 Get protocols from queryMap |
|
Kudos, SonarCloud Quality Gate passed! |
AlbumenJ
requested changes
Jun 12, 2023
Comment on lines
369
to
371
| if (t instanceof SkipFailbackWrapperException) { | ||
| throw t; | ||
| } |
AlbumenJ
approved these changes
Jun 12, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What is the purpose of the change
fix #12359
pr #12316 modifies the status when the module fails to start, thus preventing DefaultApplicationDeployer#prepareApplicationInstance from running.
In addition to exposing metadataService, prepareApplicationInstance also includes protocol extension verification. When the comsumer configures the wrong protocol, the wrong prompt is printed.
Brief changelog
There is no problem with the modification of pr #12316. We need to fail fast when building the Invoker step to verify the serialization protocol, and do not perform subsequent steps
Verifying this change
Following the steps in #12359, gives the expected result