fix(store): keep hostname store addresses

[kalayciburak]

Purpose of the PR

• close [Bug] Store IpUtil.getNearestAddress logs ERROR for hostname addresses and can return 127.0.0.1 #3255

IpUtil.getNearestAddress treats a configured hostname as a dotted IPv4. Integer.parseInt fails on the first DNS label and the catch logs that as getRaftAddress, a method that does not exist. When the only collected address is loopback, the single-address shortcut replaces the hostname with 127.0.0.1.

Main Changes

• Return a host that is not a dotted IPv4 literal unchanged, before listing local addresses, so a hostname does not log an error.
• Keep a configured IPv4 when it is bound locally, including loopback. Skip loopback and link-local only when choosing a fallback for an address that is not local.
• Name the failure log after getNearestAddress.

Verifying these changes

• Need tests and can be verified as follows:
  • JAVA_HOME=17 mvn test -pl hugegraph-store/hg-store-test -am -P store-core-test -Dtest=IpUtilTest -Dsurefire.failIfNoSpecifiedTests=false
  • RED: 127.0.0.2:8510 was 127.0.0.1:8510, and a hostname logged ERROR.
  • GREEN: IpUtilTest 3 run, 0 failed. 127.0.0.1:8510 stays when loopback is bound.

Does this PR potentially affect the following parts?

• Nope

Documentation Status

• Doc - No Need: no user-visible documentation is affected.

Documentation files in this PR or paired hugegraph-doc PR:

[bitflicker64]

Blocking: yes. Summary: The hostname early return fixes the ERROR log and keeps DNS addresses, but dropping loopback from the candidate list changes the address a Store with the default 127.0.0.1 config registers with PD, so its registered raft address no longer matches the PeerId it runs raft with. Evidence: exact-head diff of IpUtil at 853b726; old and new IpUtil compiled and run on one host (lo0 127.0.0.1, en0 10.7.6.196, utun 100.82.33.17): 127.0.0.1:8510 returned 127.0.0.1:8510 before and 100.82.33.17:8510 after; default raft.address: 127.0.0.1:8510 in hg-store-dist conf/application.yml; PartitionEngine:250, HgStoreEngine:383-388 and :406, PartitionManager:783-806 traced statically, not run end to end; latest-head workflows are waiting for maintainer approval (action_required), so CI has not run.

[bitflicker64]

Important: Skipping loopback here changes the result for the shipped default config, not only for hostnames. hg-store-dist/.../conf/application.yml sets raft.address: 127.0.0.1:8510 and grpc.host: 127.0.0.1. Before this PR, 127.0.0.1 was in the list and had distance 0, so getNearestAddress("127.0.0.1:8510") returned it unchanged. I compiled the base and head IpUtil and ran both on a host with lo0, en0 and a utun interface: base returned 127.0.0.1:8510, head returned 100.82.33.17:8510 (a VPN address). HeartbeatService.registerStore() sends that value to PD as the raft address, but PartitionEngine (line 250) still builds its own PeerId from options.getRaftAddress() = 127.0.0.1:8510. Peer lists come from the registered address (PartitionManager.getPartitionPeers / shards2Peers, used at HgStoreEngine line 406), and HgStoreEngine lines 383-388 skip creating a partition engine when the conf does not contain options.getRaftAddress(). So a default single-node Store on any machine with a network interface registers an address that does not match its raft identity, and the choice depends on interface order (Docker bridge, VPN). Please keep the configured address when it is a literal that is bound locally, including loopback. For example, return raftAddress unchanged when tmp[0] is in the local address list (collected with loopback), and drop loopback only from the fallback candidates used when the configured IPv4 is not local. Please also add a test that getNearestAddress("127.0.0.1:8510") returns 127.0.0.1:8510.

[kalayciburak]

yes, 127.0.0.1 stays when it is bound locally. loopback only drops out of the fallback list now. added a test for that, and the 127.0.0.2 case assumes a non-loopback ipv4.

[bitflicker64]

Minor: This assertion depends on the test host having a non-loopback IPv4. With only lo (for example docker run --network none, the case in #3255), getIpAddress() now returns an empty list, getNearestAddress throws "no available ipv4" internally and returns the input 127.0.0.2:8510, so assertFalse(selected.startsWith("127.")) fails. Please add Assume.assumeTrue(...) for a non-loopback IPv4 on the host, or test the selection logic with an injected address list.

[codecov]

Codecov Report

❌ Patch coverage is 72.00000% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 39.77%. Comparing base (af3c686) to head (fcdeed8).
⚠️ Report is 36 commits behind head on master.

Files with missing lines	Patch %	Lines
...n/java/org/apache/hugegraph/store/util/IpUtil.java	72.00%	3 Missing and 4 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #3291      +/-   ##
============================================
- Coverage     40.46%   39.77%   -0.69%     
+ Complexity     6566     6512      -54     
============================================
  Files           751      745       -6     
  Lines         63982    64033      +51     
  Branches       8502     8539      +37     
============================================
- Hits          25889    25472     -417     
- Misses        35175    35632     +457     
- Partials       2918     2929      +11     

☔ View full report in Codecov by Harness.
📢 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.

[bitflicker64]

Blocking: yes. Summary: The new head resolves both earlier findings: a locally bound 127.0.0.1 raft address now stays unchanged, and the loopback test is guarded by Assume. One gap remains. The bound-address check still skips link-local addresses, so a Store whose configured raft address is a bound 169.254.x.y address gets a different address registered with PD. Evidence: exact-head diff at 1b144a1; base (af3c686) and head IpUtil compiled on their own and run on one host (lo0, en0 10.7.6.196, utun 100.82.33.17): 127.0.0.1:8510, 10.7.6.196:8510 and 0.0.0.0:8510 give the same result at base and head, store0:8500 stays with no ERROR log at head, and an unbound 169.254.10.10:8510 maps to 127.0.0.1:8510 at base and 100.82.33.17:8510 at head; the bound link-local case was traced statically through getIpAddress(true) and HeartbeatService:186-187; latest-head CI jobs pass except codecov/project.

[bitflicker64]

Important: This check keeps a configured literal only if getIpAddress(true) contains it, but getIpAddress skips isLinkLocalAddress() unconditionally (lines 52-53), even when includeLoopback is true. Before this PR every IPv4 was collected, so a bound link-local literal had distance 0 and came back unchanged. Now a Store with raft.address set to a 169.254.x.y address bound on its own interface fails this check and is rewritten to the nearest other interface. HeartbeatService lines 186-187 register that rewritten value with PD, while PartitionEngine keeps options.getRaftAddress() as its PeerId. That is the same registered-address/PeerId mismatch the 127.0.0.1 fix addressed. I compiled the base and head IpUtil on their own: an unbound 169.254.10.10:8510 gives 127.0.0.1:8510 at base and 100.82.33.17:8510 at head, which confirms link-local is never a candidate at head. The bound case was traced statically, since I could not bind 169.254 without root. Please run this check against all local IPv4 addresses, link-local and loopback included, and skip link-local and loopback only from the fallback candidates. For example, give getIpAddress a single forFallback flag that drops both.

[kalayciburak]

fixed, pushed. getIpAddress now keeps link-local addresses for the bound-address check and filters link-local plus loopback only from fallback candidates. Added a regression test for a bound link-local address.

[imbajin]

Blocking: no. Summary: One minor hostname-classification edge remains for Unicode decimal digits; the inline comment gives a concrete case and fix. Evidence: Static review of isDottedIpv4 and its fallback path.

[imbajin]

🧹 Blocking: no. Summary: Character.isDigit and Integer.parseInt both accept Unicode decimal digits, so a Unicode IDN hostname such as ١.٢.٣.٤:8510 passes this check and is rewritten to a local fallback instead of being returned unchanged. Please restrict octets to ASCII 0–9 or use a strict IPv4 literal parser. Evidence: Static path through isDottedIpv4, the local-address check and fallback; both Java digit APIs use Unicode digit semantics.

[bitflicker64]

Blocking: no. Summary: The head keeps hostnames and locally bound IPv4 literals (loopback and link-local included) unchanged and only drops loopback and link-local from fallback candidates, which resolves #3255 and the earlier review findings. One minor classification gap remains in isDottedIpv4 for non-ASCII digits. Evidence: exact-head diff at f833c05 against merge base af3c686; traced getNearestAddress callers (HeartbeatService:186-187) and PeerId construction (PartitionEngine:250); confirmed on JDK that Character.isDigit('\u0661') is true and Integer.parseInt("\u0661") returns 1; IpUtilTest is added to the store-core-test include list that pd-store-ci runs; all latest-head workflows are action_required (waiting for maintainer approval), so CI has not run on this head.

[bitflicker64]

Minor: Character.isDigit accepts any Unicode decimal digit, and Integer.parseInt parses them too (checked on JDK: Character.isDigit('\u0661') is true and Integer.parseInt("\u0661") returns 1). So a host such as \u0661.\u0662.\u0663.\u0664 is classified as a dotted IPv4, misses the getIpAddress(false).contains(...) check, and is rewritten to a fallback address instead of being returned unchanged as the hostname branch intends. Please restrict octets to ASCII, for example char c = part.charAt(i); if (c < '0' || c > '9') return false;.

[kalayciburak]

fixed, restricted octets to ASCII digits and added a regression test for Unicode decimal digits. pushed fcdeed8.

[imbajin]

THX