DUBBO-7349 Fix the build on Linux ARM64 CPU architecture by martin-g · Pull Request #7408 · apache/dubbo

martin-g · 2021-03-18T11:57:07Z

What is the purpose of the change

Apache Dubbo is a Java application and thus should work fine on any architecture supported by the Java Virtual Machine.
But Dubbo depends on some libraries which use native binaries that do not have aarch64 version, for example embedded-redis and embedded-consul.
This PR updates those dependencies with newer versions which added support for ARM64 architecture!

Verifying this change

The PR re-introduces TravisCI as a build tool to build and tests master branch every night.
This has been discussed at https://lists.apache.org/thread.html/r2742c51066b55787571417ed74843cd9607ba8202566ddd807f9bdab%40%3Cdev.dubbo.apache.org%3E
Someone from Dubbo team will have to setup the Cron job at TravisCI UI !

AlbumenJ · 2021-03-18T12:37:53Z

dubbo-metadata/dubbo-metadata-report-redis/pom.xml

-            <groupId>com.github.kstyrc</groupId>
+            <groupId>com.github.codemonstur</groupId>
            <artifactId>embedded-redis</artifactId>
-            <scope>test</scope>


why remove test scope

because it is already set at https://github.com/martin-g/dubbo/blob/fix-build-and-tests-on-arm64/dubbo-dependencies-bom/pom.xml#L628 in the dependencyManagement section

this way it is in sync with

https://github.com/martin-g/dubbo/blob/fix-build-and-tests-on-arm64/dubbo-registry/dubbo-registry-multiple/pom.xml#L55-L58

https://github.com/martin-g/dubbo/blob/fix-build-and-tests-on-arm64/dubbo-registry/dubbo-registry-redis/pom.xml#L47-L50

https://github.com/martin-g/dubbo/blob/fix-build-and-tests-on-arm64/dubbo-rpc/dubbo-rpc-redis/pom.xml#L42-L45

Rather than remove <scope>test</scope>, add <scope>test</scope> will be better I think.

https://github.com/martin-g/dubbo/blob/fix-build-and-tests-on-arm64/dubbo-registry/dubbo-registry-multiple/pom.xml#L55-L58

https://github.com/martin-g/dubbo/blob/fix-build-and-tests-on-arm64/dubbo-registry/dubbo-registry-redis/pom.xml#L47-L50

https://github.com/martin-g/dubbo/blob/fix-build-and-tests-on-arm64/dubbo-rpc/dubbo-rpc-redis/pom.xml#L42-L45

@horizonzy OK, I will add the scope!

Done!
Also rebased to latest master

dubbo-dependencies-bom/pom.xml

szpak · 2021-03-18T17:37:44Z

dubbo-dependencies-bom/pom.xml

                <version>${grpc.version}</version>
            </dependency>
+            <!-- Groovy, for Embedded Consul. Could be removed once embedded-consul:2.2.1 is released
+             See https://github.com/pszymczyk/embedded-consul/pull/114


You might wait a little bit with merging that PR. We plan to release 2.2.1 "soon".

I really hope this PR can be merged ASAP, so this issue #7421 can be fixed.

@pinxiong I am afraid that this PR won't fix the issue for Mac ARM64. Currently embedded-redis:0.9.0 supports only Linux ARM64. We used Docker+QEMU to build the Linux ARM64 binary - https://github.com/codemonstur/embedded-redis/tree/master/src/main/binaries. I am not sure whether there is a similar way to emulate Mac ARM64.
I am not sure whether @codemonstur has access to Mac M1 machine to build the binary.

Thank you for letting me know! If somebody need my help for testing in Mac M1 machine, I'm very happy to do it. @martin-g @codemonstur

@pinxiong With PR codemonstur/embedded-redis#3 I've tried to convince @codemonstur to add support for a special system property to ExecutableProvider to set the path to the redis-server binary but he didn't like this approach.
If this PR was merged then Dubbo/pom.xml could add a profile for Mac ARM64 and export this property in Maven Surefire argline. Maybe @codemonstur will reconsider it.

BTW once you pass the issue with embedded-redis I expect that you will face similar issue with embedded-consul.

BTW once you pass the issue with embedded-redis I expect that you will face similar issue with embedded-consul.

@martin-g Thanks for your reminder.

@martin-g I have passed the issue with embedded-redis when I disabled all test cases in RedisMetadataReportTest. Fortunately, I didn't face similar issue with embedded-consul.

You might wait a little bit with merging that PR. We plan to release 2.2.1 "soon".

@szpak Any ETA ?

@martin-g ETA: -15 minutes :-)

https://github.com/pszymczyk/embedded-consul/releases/tag/release%2F2.2.1

Thank you, @szpak!
I've upgraded to 2.2.1 with 8825d52

@AlbumenJ The PR is ready for review and once the CI finishes green for merging!

codemonstur · 2021-03-22T14:29:49Z

...eport-redis/src/test/java/org/apache/dubbo/metadata/store/redis/RedisMetadataReportTest.java

@@ -60,15 +60,15 @@ public void constructor(TestInfo testInfo) throws IOException {
        String methodName = testInfo.getTestMethod().get().getName();


Below this line you would put

ExecutableProvider provider = () -> new File(System.getProperty("my.custom.property"));

codemonstur · 2021-03-22T14:30:24Z

...eport-redis/src/test/java/org/apache/dubbo/metadata/store/redis/RedisMetadataReportTest.java

        if ("testAuthRedisMetadata".equals(methodName) || ("testWrongAuthRedisMetadata".equals(methodName))) {
            String password = "チェリー";
-            RedisServerBuilder builder = RedisServer.builder().port(redisPort).setting("requirepass " + password);
+            RedisServerBuilder builder = RedisServer.newRedisServer().port(redisPort).setting("requirepass " + password);


Add .redisExecProvider(provider) here.

codemonstur · 2021-03-22T14:30:43Z

...eport-redis/src/test/java/org/apache/dubbo/metadata/store/redis/RedisMetadataReportTest.java

            registryUrl = URL.valueOf("redis://username:" + password + "@localhost:" + redisPort);
        } else {
-            RedisServerBuilder builder = RedisServer.builder().port(redisPort);
+            RedisServerBuilder builder = RedisServer.newRedisServer().port(redisPort);


And here add .redisExecProvider(provider) as well.

codemonstur · 2021-03-22T14:31:55Z

...eport-redis/src/test/java/org/apache/dubbo/metadata/store/redis/RedisMetadataReportTest.java

            String password = "チェリー";
-            RedisServerBuilder builder = RedisServer.builder().port(redisPort).setting("requirepass " + password);
+            RedisServerBuilder builder = RedisServer.newRedisServer().port(redisPort).setting("requirepass " + password);
            if (SystemUtils.IS_OS_WINDOWS) {


Yuck. I'm going to add a .settingIf(boolean, String) so that you can use the builder as it was intended.

@codemonstur Thanks for your help. I hope everything will be the same as you expected.

@codemonstur I see that your update has been merged in master branch. However, I still cannot work well when I run RedisMetadataReportTest.

If we want to fix issue #7421 , we need to upgrade the redis from 2.8.19 to 6.0.10 or 6.2 RC2 (see #8062, #8195) in embedded-redis

We can get some more details from redis releases 6.0.10

Bug fixes:

Fix setproctitle related crashes. (#8150, #8088)
Caused various crashes on startup, mainly on Apple M1 chips or under instrumentation

I really hope you can commit an new PR to solve this problem (#7421).

Maybe @martin-g can persuade @codemonstur to do it.

I was going to suggest to create an issue at https://github.com/codemonstur/embedded-redis and discuss there but "Issues" are not enabled there, only "Pull Requests".

@pinxiong As I said earlier - many of us do not have access to Mac ARM64 hardware and it is impossible for us to improve for it. But @codemonstur added APIs to use custom ExecutableProvider that can run any binary you tell it to.
For example, you can introduce a new Maven profile that is activated only for os=mac + arch=aarch64 and sets system property embedded.redis.executable (see https://github.com/codemonstur/embedded-redis/blob/master/src/main/java/redis/embedded/core/ExecutableProvider.java#L18). The value should be an absolute path to the Redis 6.x binary for Mac ARM64. Then in the tests code check whether this system property is non-null and use ExecutableProvider.newSystemPropertyProvider() instead of the default one.

@pinxiong the current approach that I think will work for you is as @martin-g described. I just enabled issues in https://github.com/codemonstur/embedded-redis. So if you guys want to discuss any it is now possible.

@martin-g @codemonstur I have opened an issue #6 where we can discuss.

AlbumenJ · 2021-04-06T08:30:31Z

@martin-g great! Please merge master branch to solve confilcting file. README.md is confilcted with master branch.

martin-g · 2021-04-06T08:39:45Z

Done!

codecov-io · 2021-04-06T09:17:45Z

Codecov Report

Merging #7408 (a70d2c6) into master (0f5040b) will decrease coverage by 0.05%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master    #7408      +/-   ##
============================================
- Coverage     59.24%   59.19%   -0.06%     
+ Complexity      503      501       -2     
============================================
  Files          1076     1076              
  Lines         43407    43406       -1     
  Branches       6337     6337              
============================================
- Hits          25716    25693      -23     
- Misses        14852    14877      +25     
+ Partials       2839     2836       -3

Impacted Files	Coverage Δ	Complexity Δ
.../dubbo/registry/consul/ConsulServiceDiscovery.java	`0.00% <ø> (ø)`	`0.00 <0.00> (ø)`
...che/dubbo/remoting/transport/mina/MinaChannel.java	`35.52% <0.00%> (-10.53%)`	`14.00% <0.00%> (-1.00%)`
...dubbo/remoting/exchange/support/DefaultFuture.java	`80.34% <0.00%> (-6.84%)`	`0.00% <0.00%> (ø%)`
...he/dubbo/remoting/transport/netty/NettyServer.java	`70.17% <0.00%> (-3.51%)`	`8.00% <0.00%> (-1.00%)`
...e/dubbo/registry/integration/RegistryProtocol.java	`58.35% <0.00%> (-2.84%)`	`0.00% <0.00%> (ø%)`
...e/dubbo/remoting/transport/netty4/NettyClient.java	`69.56% <0.00%> (-1.45%)`	`0.00% <0.00%> (ø%)`
...pache/dubbo/registry/support/FailbackRegistry.java	`65.42% <0.00%> (-0.94%)`	`0.00% <0.00%> (ø%)`
...apache/dubbo/common/extension/ExtensionLoader.java	`77.95% <0.00%> (-0.45%)`	`0.00% <0.00%> (ø%)`
...n/java/org/apache/dubbo/common/utils/UrlUtils.java	`74.14% <0.00%> (-0.35%)`	`0.00% <0.00%> (ø%)`
...ubbo/registry/client/ServiceDiscoveryRegistry.java	`42.85% <0.00%> (+0.59%)`	`0.00% <0.00%> (ø%)`
... and 3 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 0f5040b...a70d2c6. Read the comment docs.

AlbumenJ

LGTM.

Update embedded-redis and embedded-consul to newer versions which have native binaries for aarch64 Re-introduce TravisCI as a build tool only for ARM64. It will run as a cron job every night

… section Inline the version so that it is not forgotten to be removed when the dependency is removed with the upgrade to embedded-consul:2.2.1 See pszymczyk/embedded-consul#114

horizonzy

LGTM.

codecov-commenter · 2026-04-01T03:55:59Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 59.19%. Comparing base (0f5040b) to head (a70d2c6).

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #7408      +/-   ##
============================================
- Coverage     59.24%   59.19%   -0.06%     
+ Complexity      503      501       -2     
============================================
  Files          1076     1076              
  Lines         43407    43406       -1     
  Branches       6337     6337              
============================================
- Hits          25716    25693      -23     
- Misses        14852    14877      +25     
+ Partials       2839     2836       -3

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

AlbumenJ reviewed Mar 18, 2021

View reviewed changes

szpak reviewed Mar 18, 2021

View reviewed changes

kevinw66 mentioned this pull request Mar 21, 2021

Can't start redis server. Check logs for details. #7421

Closed

2 tasks

martin-g changed the title ~~DUBBO-7349 Fix the build on ARM64 CPU architecture~~ DUBBO-7349 Fix the build on Linux ARM64 CPU architecture Mar 22, 2021

codemonstur reviewed Mar 22, 2021

View reviewed changes

AlbumenJ approved these changes Apr 10, 2021

View reviewed changes

martin-g added 5 commits April 12, 2021 08:43

Issue #7349 - Fix the build on ARM64 CPU architecture

20635e0

Update embedded-redis and embedded-consul to newer versions which have native binaries for aarch64 Re-introduce TravisCI as a build tool only for ARM64. It will run as a cron job every night

Issue #7408 - Set the scope of groovy-xml in the dependencyManagement…

d0fab31

… section Inline the version so that it is not forgotten to be removed when the dependency is removed with the upgrade to embedded-consul:2.2.1 See pszymczyk/embedded-consul#114

Update embedded-redis to 0.10.0

f1678da

DUBBO-7349 Upgrade Embedded-Consul to 2.2.1

bcaf9b9

DUBBO-7349 Add scope=test to all occurrences of embedded-[redis|consul]

a70d2c6

horizonzy approved these changes Apr 12, 2021

View reviewed changes

martin-g mentioned this pull request Apr 14, 2021

DUBBO-7349 Add a note that all Pull Requests are tested at GitHub Actions apache/dubbo-website#775

Merged

AlbumenJ merged commit 062166f into apache:master Apr 15, 2021

CrazyHZM mentioned this pull request Sep 30, 2021

Dubbo framework support for ARM architecture discussion #7349

Closed

martin-g deleted the fix-build-and-tests-on-arm64 branch September 30, 2021 07:33

		@@ -60,15 +60,15 @@ public void constructor(TestInfo testInfo) throws IOException {
		String methodName = testInfo.getTestMethod().get().getName();

Conversation

martin-g commented Mar 18, 2021

What is the purpose of the change

Verifying this change

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

szpak Mar 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codemonstur Mar 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

martin-g Apr 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlbumenJ commented Apr 6, 2021

Uh oh!

martin-g commented Apr 6, 2021

Uh oh!

codecov-io commented Apr 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

AlbumenJ left a comment

Choose a reason for hiding this comment

Uh oh!

horizonzy left a comment

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Apr 1, 2026

Codecov Report

Uh oh!

Reviewers

szpak Mar 18, 2021 •

edited

Loading

codemonstur Mar 22, 2021 •

edited

Loading

martin-g Apr 16, 2021 •

edited

Loading

codecov-io commented Apr 6, 2021 •

edited

Loading