-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
[ISSUE#11659] Develop config query chain of responsibility. #12892
base: develop
Are you sure you want to change the base?
[ISSUE#11659] Develop config query chain of responsibility. #12892
Conversation
modified: config/src/main/java/com/alibaba/nacos/config/server/controller/ConfigController.java modified: config/src/main/java/com/alibaba/nacos/config/server/controller/ConfigServletInner.java modified: config/src/main/java/com/alibaba/nacos/config/server/controller/v2/ConfigControllerV2.java new file: config/src/main/java/com/alibaba/nacos/config/server/enums/ApiVersionEnum.java new file: config/src/main/java/com/alibaba/nacos/config/server/model/ConfigQueryChainRequest.java new file: config/src/main/java/com/alibaba/nacos/config/server/model/ConfigQueryChainResponse.java modified: config/src/main/java/com/alibaba/nacos/config/server/remote/ConfigQueryRequestHandler.java new file: config/src/main/java/com/alibaba/nacos/config/server/remote/query/ConfigQueryChainService.java new file: config/src/main/java/com/alibaba/nacos/config/server/remote/query/ConfigQueryHandlerChain.java new file: config/src/main/java/com/alibaba/nacos/config/server/remote/query/ConfigQueryHandlerChainBuilder.java new file: config/src/main/java/com/alibaba/nacos/config/server/remote/query/DefaultConfigQueryHandlerChainBuilder.java new file: config/src/main/java/com/alibaba/nacos/config/server/remote/query/handler/AbstractConfigQueryHandler.java new file: config/src/main/java/com/alibaba/nacos/config/server/remote/query/handler/ConfigChainEntryHandler.java new file: config/src/main/java/com/alibaba/nacos/config/server/remote/query/handler/ConfigQueryHandler.java new file: config/src/main/java/com/alibaba/nacos/config/server/remote/query/handler/FormalHandler.java new file: config/src/main/java/com/alibaba/nacos/config/server/remote/query/handler/GrayRuleMatchHandler.java modified: config/src/main/java/com/alibaba/nacos/config/server/controller/ConfigController.java modified: config/src/main/java/com/alibaba/nacos/config/server/controller/ConfigServletInner.java modified: config/src/main/java/com/alibaba/nacos/config/server/controller/v2/ConfigControllerV2.java new file: config/src/main/java/com/alibaba/nacos/config/server/enums/ApiVersionEnum.java new file: config/src/main/java/com/alibaba/nacos/config/server/model/ConfigQueryChainRequest.java new file: config/src/main/java/com/alibaba/nacos/config/server/model/ConfigQueryChainResponse.java modified: config/src/main/java/com/alibaba/nacos/config/server/remote/ConfigQueryRequestHandler.java new file: config/src/main/java/com/alibaba/nacos/config/server/remote/query/ConfigQueryChainService.java new file: config/src/main/java/com/alibaba/nacos/config/server/remote/query/ConfigQueryHandlerChain.java new file: config/src/main/java/com/alibaba/nacos/config/server/remote/query/ConfigQueryHandlerChainBuilder.java new file: config/src/main/java/com/alibaba/nacos/config/server/remote/query/DefaultConfigQueryHandlerChainBuilder.java new file: config/src/main/java/com/alibaba/nacos/config/server/remote/query/handler/AbstractConfigQueryHandler.java new file: config/src/main/java/com/alibaba/nacos/config/server/remote/query/handler/ConfigChainEntryHandler.java new file: config/src/main/java/com/alibaba/nacos/config/server/remote/query/handler/ConfigQueryHandler.java new file: config/src/main/java/com/alibaba/nacos/config/server/remote/query/handler/FormalHandler.java new file: config/src/main/java/com/alibaba/nacos/config/server/remote/query/handler/GrayRuleMatchHandler.java new file: config/src/main/java/com/alibaba/nacos/config/server/remote/query/handler/TagNotFoundHandler.java new file: config/src/main/resources/META-INF/services/com.alibaba.nacos.config.server.remote.query.ConfigQueryHandlerChainBuilder modified: config/src/test/java/com/alibaba/nacos/config/server/controller/ConfigServletInnerTest.java modified: config/src/test/java/com/alibaba/nacos/config/server/controller/v2/ConfigControllerV2Test.java modified: config/src/test/java/com/alibaba/nacos/config/server/remote/ConfigQueryRequestHandlerTest.java
Thanks for your this PR. 🙏 感谢您提交的PR。 🙏 |
if (StringUtils.isNotBlank(tag)) { | ||
appLabels.put(TagGrayRule.VIP_SERVER_TAG_LABEL, tag); | ||
} else { | ||
appLabels = new HashMap<>(meta.getAppLabels()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
appLabels.putAll(meta.getAppLabels());
public enum ConfigQueryStatus { | ||
BETA, | ||
TAG, | ||
TAG_NOT_FOUND, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
beta,tag-> gray
private final ConfigQueryHandlerChain configQueryHandlerChain; | ||
|
||
public ConfigQueryChainService() { | ||
Collection<ConfigQueryHandlerChainBuilder> configQueryHandlerChainBuilders = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ConfigQueryHandlerChainBuilder should load multi , but use only one with specific name which's default value is nacos.
configQueryHandlerChain = builder.addHandler(new ConfigChainEntryHandler()) | ||
.addHandler(new GrayRuleMatchHandler()) | ||
.addHandler(new TagNotFoundHandler()) | ||
.addHandler(new FormalHandler()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be extracted to DefaultConfigQueryHandlerChainBuilder with name of 'nacos.'
|
||
private static final String GRAY_RULE_MATCH_HANDLER = "grayRuleMatchHandler"; | ||
|
||
private ConfigCacheGray matchedGray; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not Thread Safe.
Every handler should be stateless.
response.setStatus(ConfigQueryChainResponse.ConfigQueryStatus.BETA); | ||
} else if (TagGrayRule.TYPE_TAG.equals(matchedGray.getGrayRule().getType())) { | ||
response.setStatus(ConfigQueryChainResponse.ConfigQueryStatus.TAG); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
set match gray only, ignore beta& tag
* | ||
* @author Nacos | ||
*/ | ||
public class TagNotFoundHandler extends AbstractConfigQueryHandler { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
change to SpecificTagConfigQueryHandler
delayed, clientIp, notify, "grpc"); | ||
|
||
return response; | ||
|
||
} catch (Exception e) { | ||
return ConfigQueryResponse.buildFailResponse(ResponseCode.FAIL.getCode(), e.getMessage()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add error log to nacos.log
|
||
@Override | ||
public ConfigQueryHandlerChain build() { | ||
return new ConfigQueryHandlerChain(head); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ConfigQueryHandlerChain chain =new ConfigQueryHandlerChain();
chain.addHandler(new ConfigChainEntryHandler())
.addHandler(new GrayRuleMatchHandler())
.addHandler(new TagNotFoundHandler())
.addHandler(new FormalHandler());
return chain;
tail.setNextHandler(handler); | ||
tail = handler; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move to ConfigQueryHandlerChain#addHandler
* @param handler the handler to be added | ||
* @return the current builder instance | ||
*/ | ||
ConfigQueryHandlerChainBuilder addHandler(ConfigQueryHandler handler); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move to ConfigQueryHandlerChain#addHandler
|
||
private static final Logger LOGGER = LoggerFactory.getLogger(AbstractConfigQueryHandler.class); | ||
|
||
public ConfigQueryHandler nextHandler; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add prevHandler
|
* @author Nacos | ||
*/ | ||
public class ConfigQueryHandlerChain { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这些类迁移到service包下,不应该放在remote下
return configQueryHandlerChain.handle(request); | ||
} catch (Exception e) { | ||
LOGGER.error("[Error] Fail to handle ConfigQueryChainRequest", e); | ||
return new ConfigQueryChainResponse(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should return error response with error info
* Gets the name of the handler. | ||
* @return The name of the handler. | ||
*/ | ||
String getQueryHandlerName(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
change to getName()
* @param meta the request meta | ||
* @return the constructed ConfigQueryChainRequest object | ||
*/ | ||
public ConfigQueryChainRequest buildChainRequest(ConfigQueryRequest request, RequestMeta meta) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extract a new spi component which named ConfigQueryChainRequestExtractor, with two method.
ConfigQueryChainRequest extract(HttpServerletRequest request) , ConfigQueryChainRequest extract(ConfigQueryRequest request, RequestMeta requestMeta)
it closes #11659
What is the purpose of the change
XXXXX
Brief changelog
XX
Verifying this change
XXXX
Follow this checklist to help us incorporate your contribution quickly and easily:
[ISSUE #123] Fix UnknownException when host config not exist
. Each commit in the pull request should have a meaningful subject line and body.mvn -B clean package apache-rat:check findbugs:findbugs -Dmaven.test.skip=true
to make sure basic checks pass. Runmvn clean install -DskipITs
to make sure unit-test pass. Runmvn clean test-compile failsafe:integration-test
to make sure integration-test pass.