fix: ClassCastException when follower forwards positional-param command to leader#3961
fix: ClassCastException when follower forwards positional-param command to leader#3961
Conversation
…nd to leader
When a Raft follower forwarded a write command with positional parameters
(e.g. INSERT with ?-placeholders), the params were serialized as a JSON array
[110]. The leader's PostCommandHandler called json.toMap(true), which converted
the numeric array to float[]{110.0} via the vector-embedding optimization
introduced in #3864. The subsequent cast (Map<String,Object>) float[] then threw
ClassCastException: class [F cannot be cast to class java.util.Map.
Fix 1 (RaftReplicatedDatabase): serialize positional args as an ordinal map
{"0": v0, "1": v1, ...} instead of a JSON array, matching the canonical format
produced by RemoteDatabase.mapArgs() and expected by mapParams() on the leader.
Fix 2 (PostCommandHandler): defensive handling for the case where params is a
List or primitive array, converting it to an ordinal map so the handler is
robust regardless of how params arrive.
Regression test added to RaftLeaderProxyIT.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 25 |
🟢 Coverage 36.84% diff coverage · -7.25% coverage variation
Metric Results Coverage variation ✅ -7.25% coverage variation Diff coverage ✅ 36.84% diff coverage Coverage variation details
Coverable lines Covered lines Coverage Common ancestor commit (bd33f49) 118934 86428 72.67% Head commit (df6b830) 150526 (+31592) 98474 (+12046) 65.42% (-7.25%) Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch:
<coverage of head commit> - <coverage of common ancestor commit>Diff coverage details
Coverable lines Covered lines Diff coverage Pull request (#3961) 19 7 36.84% Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified:
<covered lines added or modified>/<coverable lines added or modified> * 100%
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes. Give us feedback
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses a ClassCastException that occurs when positional parameters are proxied to the Raft leader. The fix involves serializing positional arguments as an ordinal map in RaftReplicatedDatabase and updating PostCommandHandler to robustly handle parameters provided as Maps, Lists, or primitive arrays. A regression test has been added to verify the fix. Feedback suggests enhancing the regression test to specifically include an all-numeric array to trigger the float[] optimization path and optimizing HashMap initializations in PostCommandHandler by accounting for the default load factor.
| final JSONObject body = new JSONObject() | ||
| .put("language", "sql") | ||
| .put("command", "INSERT INTO V1 SET id = ?, name = ?") | ||
| .put("params", new com.arcadedb.serializer.json.JSONArray().put(42).put("positional-test")); |
There was a problem hiding this comment.
The current regression test uses a mixed-type array [42, "positional-test"]. While this correctly tests the List handling in PostCommandHandler, it does not trigger the float[] optimization in json.toMap(true) which was the specific cause of the ClassCastException mentioned in the PR description (where an all-numeric array like [110] is converted to a primitive array).
Consider adding a second test case or updating this one to use an all-numeric array to ensure the float[] code path in PostCommandHandler is also covered by the regression suite.
There was a problem hiding this comment.
Added in df6b830. The test now sends a second request with params: [42] (all-numeric) via the follower, which exercises the float[] branch in PostCommandHandler. The assertion verifies the response is 200 rather than a 500 ClassCastException.
| paramMap = new HashMap<>(list.size()); | ||
| for (int i = 0; i < list.size(); i++) | ||
| paramMap.put("" + i, list.get(i)); |
There was a problem hiding this comment.
When initializing a HashMap with a known size, it is recommended to account for the default load factor (0.75) to avoid an immediate resize when the map is filled. For a list of size n, the initial capacity should be (int) (n / 0.75f) + 1.
| paramMap = new HashMap<>(list.size()); | |
| for (int i = 0; i < list.size(); i++) | |
| paramMap.put("" + i, list.get(i)); | |
| paramMap = new HashMap<>((int) (list.size() / 0.75f) + 1); | |
| for (int i = 0; i < list.size(); i++) | |
| paramMap.put("" + i, list.get(i)); |
| paramMap = new HashMap<>(len); | ||
| for (int i = 0; i < len; i++) | ||
| paramMap.put("" + i, java.lang.reflect.Array.get(rawParams, i)); |
There was a problem hiding this comment.
Similar to the List case, the HashMap for primitive arrays should be initialized with a capacity that accounts for the load factor to prevent unnecessary resizing.
| paramMap = new HashMap<>(len); | |
| for (int i = 0; i < len; i++) | |
| paramMap.put("" + i, java.lang.reflect.Array.get(rawParams, i)); | |
| paramMap = new HashMap<>((int) (len / 0.75f) + 1); | |
| for (int i = 0; i < len; i++) | |
| paramMap.put("" + i, java.lang.reflect.Array.get(rawParams, i)); |
…aces Three related issues fixed in Docker-based Raft HA tests: - DatabaseWrapper.createDatabase(): restore FIXED strategy to prevent the client from following cluster-reported internal Docker addresses (arcadedb-N:2480) after failures. Broaden the retry loop to cover both ServerIsNotTheLeaderException with null leader (post-election window) and general connection failures (server temporarily down during rolling restart). - ContainersTestTemplate: add waitForAllNodesKnowLeader() helper that polls /api/v1/cluster on all nodes until every one reports a non-null leaderHttpAddress. Bridges the window where one node is already leader but followers haven't yet propagated the leader address. - SplitBrainIT.splitBrainPrevention(): call waitForAllNodesKnowLeader() after waitForRaftLeader() before issuing writes, and increase the replication check timeout from 30s to 2 minutes. Without the leader-address wait, addUserAndPhotos() silently dropped writes because the follower's forwardCommandToLeaderViaRaft() failed with a null leader address. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…s from prior test run GroupManagementIT creates testuser without cleanup; FileUtils.deleteRecursively silently fails to delete ./target/config/, leaving stale server-users.jsonl that causes CREATE USER to return 403 on the next run. Dropping the users at test start makes the test idempotent against leftover state. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The bd33f49 commit added FileUtils.deleteRecursively("./target/config/") to TestServerHelper.deleteDatabaseFolders(), which runs between setTestConfiguration() and startServers() in @beforeeach. This wiped the gremlin-server.yaml, gremlin-server.properties, and gremlin-server.groovy files before the server could read them, so the Gremlin plugin fell back to default settings with no graphs and no traversal sources, causing "traversal source [graph] not configured" failures. Fix: move config file writing to onBeforeStarting(), which is called inside startServers() after folder cleanup but before server.start(). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…verage - Use (int)(n/0.75f)+1 initial capacity for the List<> and primitive-array positional-param HashMaps to avoid rehashing when the map is filled - Extend positionalParamsViaFollowerIsProxiedToLeader to also send an all-numeric array [42] via a follower, exercising the float[] code path that was the original ClassCastException trigger Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3961 +/- ##
==========================================
+ Coverage 63.84% 64.44% +0.60%
==========================================
Files 1591 1593 +2
Lines 118934 119287 +353
Branches 25275 25384 +109
==========================================
+ Hits 75929 76872 +943
+ Misses 32536 31755 -781
- Partials 10469 10660 +191 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
1 participant
Summary
INSERT INTO User SET id = ?), the params were serialized as a JSON array[110]. The leader'sPostCommandHandlercalledjson.toMap(true), which converted the all-numeric array tofloat[]{110.0}via the vector-embedding optimization from Cypher Batch creation is slow with vector indexes #3864. The subsequent cast(Map<String,Object>) float[]threwClassCastException: class [F cannot be cast to class java.util.Map.RaftReplicatedDatabase): serialize positional args as an ordinal map{"0": v0, "1": v1, ...}instead of a plain JSON array, matching the canonical format produced byRemoteDatabase.mapArgs()and expected bymapParams()on the leader.PostCommandHandler): defensive handling for whenparamsis aListor primitive array - converts to ordinal map, making the handler robust regardless of how params arrive.Test Plan
positionalParamsViaFollowerIsProxiedToLeaderinRaftLeaderProxyIT- sends a parameterized INSERT via a follower's HTTP endpoint, verifies the record is committed and replicated to all nodesRaftLeaderProxyITpass (ddlViaFollowerIsProxiedToLeader,dmlViaFollowerIsProxiedToLeader,positionalParamsViaFollowerIsProxiedToLeader)🤖 Generated with Claude Code