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

[ISSUE #11994]修复HttpRequestInstanceBuilderTest出现的空指针问题,优化SnowFlakeInstanceIdGenerator初始化流程 #11995

Merged
merged 3 commits into from
Apr 23, 2024

Conversation

llzcx
Copy link
Contributor

@llzcx llzcx commented Apr 22, 2024

Please do not create a Pull Request without creating an issue first.

What is the purpose of the change

  1. fix unit test bug.

  2. 解决SnowFlakeInstanceIdGenerator类加载时强依赖workerId的初始化过程。

Brief changelog

@BeforeClass增加EnvUtil.setEnvironment(new StandardEnvironment());

改变WorkerId的加载方式。通过SnowFlakeInstanceIdGenerator静态代码块加载->在generateInstanceId通过双检锁保证加载一次

Verifying this change

XXXX

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

  • Make sure there is a Github issue filed for the change (usually before you start working on it). Trivial changes like typos do not require a Github issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
  • Format the pull request title like [ISSUE #123] Fix UnknownException when host config not exist. Each commit in the pull request should have a meaningful subject line and body.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Write necessary unit-test to verify your logic correction, more mock a little better when cross module dependency exist. If the new feature or significant change is committed, please remember to add integration-test in test module.
  • Run mvn -B clean package apache-rat:check findbugs:findbugs -Dmaven.test.skip=true to make sure basic checks pass. Run mvn clean install -DskipITs to make sure unit-test pass. Run mvn clean test-compile failsafe:integration-test to make sure integration-test pass.

@CLAassistant
Copy link

CLAassistant commented Apr 22, 2024

CLA assistant check
All committers have signed the CLA.

KomachiSion
KomachiSion previously approved these changes Apr 22, 2024
@KomachiSion
Copy link
Collaborator

Error: /home/runner/work/nacos/nacos/naming/src/main/java/com/alibaba/nacos/naming/pojo/instance/SnowFlakeInstanceIdGenerator.java:37:33: Name 'lock' must match pattern '^[A-Z][A-Z0-9](_[A-Z0-9]+)$'. [ConstantName]
Error: /home/runner/work/nacos/nacos/naming/src/main/java/com/alibaba/nacos/naming/pojo/instance/SnowFlakeInstanceIdGenerator.java:39:5: Missing a Javadoc comment. [MissingJavadocMethod]

Please fix code style problem.

@llzcx
Copy link
Contributor Author

llzcx commented Apr 22, 2024

@KomachiSion I have fixed it.

@llzcx llzcx changed the title 修复HttpRequestInstanceBuilderTest出现的空指针问题,优化SnowFlakeInstanceIdGenerator初始化流程 [ISSUE #11994]修复HttpRequestInstanceBuilderTest出现的空指针问题,优化SnowFlakeInstanceIdGenerator初始化流程 Apr 22, 2024
@@ -54,6 +56,7 @@ public class HttpRequestInstanceBuilderTest {
@BeforeClass
public static void setUpBeforeClass() {
NacosServiceLoader.load(InstanceExtensionHandler.class);
EnvUtil.setEnvironment(new StandardEnvironment());
Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM

@llzcx
Copy link
Contributor Author

llzcx commented Apr 22, 2024

Continuous Integration/CI (Ubuntu latest, 8) results:
image

2024-04-22T11:11:48.3742755Z [ERROR] testIsNeedReloadConfiguration(com.alibaba.nacos.client.logging.log4j2.Log4J2NacosLoggingAdapterTest)  Time elapsed: 0.097 s  <<< FAILURE!
2024-04-22T11:11:48.3744562Z java.lang.AssertionError
2024-04-22T11:11:48.3745214Z 	at org.junit.Assert.fail(Assert.java:87)
2024-04-22T11:11:48.3745875Z 	at org.junit.Assert.assertTrue(Assert.java:42)
2024-04-22T11:11:48.3746575Z 	at org.junit.Assert.assertTrue(Assert.java:53)
2024-04-22T11:11:48.3748197Z 	at com.alibaba.nacos.client.logging.log4j2.Log4J2NacosLoggingAdapterTest.testIsNeedReloadConfiguration(Log4J2NacosLoggingAdapterTest.java:89)
2024-04-22T11:11:48.3749966Z 	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
2024-04-22T11:11:48.3751092Z 	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
2024-04-22T11:11:48.3752439Z 	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
2024-04-22T11:11:48.3753563Z 	at java.lang.reflect.Method.invoke(Method.java:498)
2024-04-22T11:11:48.3754619Z 	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
2024-04-22T11:11:48.3755999Z 	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
2024-04-22T11:11:48.3757433Z 	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
2024-04-22T11:11:48.3758885Z 	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
2024-04-22T11:11:48.3760337Z 	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
2024-04-22T11:11:48.3761803Z 	at org.mockito.internal.runners.DefaultInternalRunner$1$1.evaluate(DefaultInternalRunner.java:54)
2024-04-22T11:11:48.3763227Z 	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
2024-04-22T11:11:48.3764471Z 	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
2024-04-22T11:11:48.3765712Z 	at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
2024-04-22T11:11:48.3767013Z 	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
2024-04-22T11:11:48.3768263Z 	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
2024-04-22T11:11:48.3769688Z 	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
2024-04-22T11:11:48.3770752Z 	at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
2024-04-22T11:11:48.3771771Z 	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
2024-04-22T11:11:48.3772849Z 	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
2024-04-22T11:11:48.3774167Z 	at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
2024-04-22T11:11:48.3775174Z 	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
2024-04-22T11:11:48.3776446Z 	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
2024-04-22T11:11:48.3777455Z 	at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
2024-04-22T11:11:48.3778645Z 	at org.mockito.internal.runners.DefaultInternalRunner$1.run(DefaultInternalRunner.java:99)
2024-04-22T11:11:48.3780414Z 	at org.mockito.internal.runners.DefaultInternalRunner.run(DefaultInternalRunner.java:105)
2024-04-22T11:11:48.3781739Z 	at org.mockito.internal.runners.StrictRunner.run(StrictRunner.java:40)
2024-04-22T11:11:48.3782932Z 	at org.mockito.junit.MockitoJUnitRunner.run(MockitoJUnitRunner.java:163)
2024-04-22T11:11:48.3784463Z 	at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:365)
2024-04-22T11:11:48.3786006Z 	at org.apache.maven.surefire.junit4.JUnit4Provider.executeWithRerun(JUnit4Provider.java:272)
2024-04-22T11:11:48.3787589Z 	at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:236)
2024-04-22T11:11:48.3789100Z 	at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:159)
2024-04-22T11:11:48.3790764Z 	at org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:386)
2024-04-22T11:11:48.3792493Z 	at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:323)
2024-04-22T11:11:48.3793933Z 	at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:143)

Log4J2NacosLoggingAdapter.isNeedReloadConfiguration
    public boolean isNeedReloadConfiguration() {
        final LoggerContext loggerContext = (LoggerContext) LogManager.getContext(false);
        final Configuration contextConfiguration = loggerContext.getConfiguration();
        for (Map.Entry<String, Appender> entry : contextConfiguration.getAppenders().entrySet()) {
            if (APPENDER_MARK.equals(entry.getValue().getName())) {
                return false;
            }
        }
        return true;
    }

image

Can the local test pass, but Continuous Integration/CI (Ubuntu latest, 8) cannot?

@KomachiSion
Copy link
Collaborator

No problem, I will fix it in next PR.

@KomachiSion KomachiSion merged commit 720519b into alibaba:develop Apr 23, 2024
6 of 7 checks passed
syshenyao pushed a commit to syshenyao/nacos that referenced this pull request Apr 25, 2024
…lakeInstanceIdGenerator初始化流程 (alibaba#11995)

* [ISSUE alibaba#11994] fix: Initialize EnvUtil's variable environment in HttpRequestInstanceBuilderTest

* refactor: Optimize the initialization process of SnowFlakeInstanceIdGenerator

* refactor: optimize the initialization process of SnowFlakeInstanceIdGenerator.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants