Skip to content

Commit ef67091

Browse files
committed
Merge branch 'upstream-main'
2 parents c267052 + 8cd72b1 commit ef67091

51 files changed

Lines changed: 1850 additions & 142 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

bolt/pom.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
<parent>
2626
<groupId>com.arcadedb</groupId>
2727
<artifactId>arcadedb-parent</artifactId>
28-
<version>26.6.1-SNAPSHOT</version>
28+
<version>26.6.1</version>
2929
<relativePath>../pom.xml</relativePath>
3030
</parent>
3131

console/pom.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
<parent>
2626
<groupId>com.arcadedb</groupId>
2727
<artifactId>arcadedb-parent</artifactId>
28-
<version>26.6.1-SNAPSHOT</version>
28+
<version>26.6.1</version>
2929
<relativePath>../pom.xml</relativePath>
3030
</parent>
3131

coverage/pom.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
<parent>
2727
<groupId>com.arcadedb</groupId>
2828
<artifactId>arcadedb-parent</artifactId>
29-
<version>26.6.1-SNAPSHOT</version>
29+
<version>26.6.1</version>
3030
<relativePath>../pom.xml</relativePath>
3131
</parent>
3232

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
# Issue #4451 - Configurable Server Log Directory
2+
3+
## Goal
4+
5+
Allow the server's file-log directory to be configured via
6+
`arcadedb.server.logsDirectory` (or the `ARCADEDB_LOG_DIR` environment variable),
7+
enabling Kubernetes deployments with `readOnlyRootFilesystem` to write logs to a
8+
writable mount instead of crashing on a read-only working directory.
9+
10+
## Problem
11+
12+
The packaged `arcadedb-log.properties` hardcodes the JUL FileHandler pattern as
13+
`./log/arcadedb.log`. The Java logging framework resolves that path relative to the
14+
process working directory the first time any log call fires - which happens during
15+
`GlobalConfiguration` static initialization, before the server root path is even set.
16+
On a read-only root filesystem the working directory is not writable, so JUL fails to
17+
open the log file:
18+
19+
```
20+
java.nio.file.FileSystemException: ./log/arcadedb.log.0.lck: Read-only file system
21+
```
22+
23+
The directory cannot simply be tied to `arcadedb.server.rootPath`, because that value
24+
is still unset at the moment logging initializes. It must be resolvable from the
25+
environment alone.
26+
27+
## Solution
28+
29+
Resolve `${...}` placeholders in the FileHandler pattern inside `DefaultLogger` - the
30+
one place ArcadeDB controls the configuration stream before handing it to JUL - using
31+
the existing `SystemVariableResolver` (system property, then environment variable, then
32+
`GlobalConfiguration`), defaulting to `./log` so existing behavior is unchanged.
33+
34+
The packaged pattern becomes:
35+
`java.util.logging.FileHandler.pattern=${arcadedb.server.logsDirectory}/arcadedb.log`
36+
37+
Resolution is applied both when pre-creating the log directory and in the pattern handed
38+
to JUL, so the two agree. A pattern with no placeholder (e.g. an absolute literal path) is
39+
returned unchanged.
40+
41+
Operators relocate the log directory by setting the environment variable:
42+
`ARCADEDB_LOG_DIR=/var/lib/arcadedb/log` (the launch scripts forward it as
43+
`-Darcadedb.server.logsDirectory=...`), or by setting the system property directly.
44+
When unset, logs default to `./log`, identical to prior releases.
45+
46+
## Affected Files
47+
48+
- `engine/src/main/java/com/arcadedb/log/DefaultLogger.java` - `resolveConfigurableLogDir` helper, wired into `createLogDirectoryFromConfig()` and `installCustomFormatter()`
49+
- `engine/src/main/java/com/arcadedb/GlobalConfiguration.java` - new `SERVER_LOGS_DIRECTORY` config entry (`arcadedb.server.logsDirectory`, default `./log`)
50+
- `engine/src/test/java/com/arcadedb/LoggerTest.java` - placeholder resolution + integration regression tests
51+
- `engine/src/test/java/com/arcadedb/GlobalConfigurationTest.java` - config entry test
52+
- `package/src/main/config/arcadedb-log.properties` - FileHandler pattern uses the placeholder
53+
- `package/src/main/scripts/server.sh`, `package/src/main/scripts/server.bat` - forward `ARCADEDB_LOG_DIR`
54+
55+
## Verification
56+
57+
- `LoggerTest` (8 tests) and `GlobalConfigurationTest` (10 tests): all PASS. Includes the
58+
issue-#3732 regression tests, which prove literal patterns (the default `./log` behavior)
59+
are unchanged.
60+
- Read-only-CWD smoke test against the real packaged `arcadedb-log.properties`: with a
61+
read-only `./log` in the working directory and `arcadedb.server.logsDirectory` pointing at
62+
a writable directory, the server logs to the writable directory, leaves `./log` untouched,
63+
and does not throw the `Read-only file system` exception.
64+
65+
## Backward Compatibility
66+
67+
The default remains `./log`, so single-host and standard Docker deployments (where the
68+
working directory is writable) behave exactly as before. The change is additive.
Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
# Issue #4468 - SQL `IN :param` with a collection parameter returns no rows when an index is used
2+
3+
## Summary
4+
5+
SQL queries using `IN :param` (named parameter) or `IN ?` (positional parameter) with a collection
6+
value return no rows when the field has an index and the index path is chosen by the planner.
7+
8+
The same collection works on the non-indexed path (`WHERE (field + 0) IN :param`) and the literal
9+
form (`WHERE field IN [1, 2, 3]`) works without restriction.
10+
11+
## Root Cause
12+
13+
`PostCommandHandler.PostCommandHandler` uses `json.toMap(true)` to deserialize the HTTP request body.
14+
The `true` flag enables an optimized path that converts JSON numeric arrays (`[1, 2, 3]`) to
15+
primitive arrays (`long[]`, `double[]`) rather than `List<Long>` to reduce boxing allocations.
16+
17+
When a parameter value like `codes = [1, 2, 3]` arrives at the server, the deserialized map entry
18+
is `"codes" -> long[]`.
19+
20+
`FetchFromIndexStep.cartesianProduct()` is responsible for expanding a multi-value expression into
21+
one index lookup per element. Before this fix its check was:
22+
23+
```java
24+
if (value instanceof Iterable<?> iterable && !(value instanceof Identifiable)) {
25+
```
26+
27+
Primitive arrays (`long[]`, `int[]`, `Object[]`) are **not** `Iterable<?>` in Java.
28+
The condition was false, the `else` branch ran, and the whole array was passed as a single index
29+
key - matching nothing.
30+
31+
The non-indexed evaluator uses `MultiValue.getMultiValueIterable()` which handles arrays correctly,
32+
so `WHERE (field + 0) IN :param` worked fine.
33+
34+
## Fix
35+
36+
`engine/src/main/java/com/arcadedb/query/sql/executor/FetchFromIndexStep.java`
37+
38+
Replaced the `Iterable<?>`-only predicate in `cartesianProduct()` with the existing `MultiValue`
39+
utility, which already detects and iterates every array type (`long[]`, `int[]`, `double[]`,
40+
`Object[]`) as well as `Collection`/`Iterable`. This is the same utility the sibling
41+
`processInCondition()` path uses (lines 258-261 of the same file), so both multi-value detection
42+
sites now share one code path. `MultiValue.isMultiValue(null)` is internally null-safe, so no
43+
explicit null guard is needed.
44+
45+
```java
46+
// Before
47+
if (value instanceof Iterable<?> iterable && !(value instanceof Identifiable)) {
48+
for (final Object elemInKey : iterable) {
49+
50+
// After
51+
if (!(value instanceof Identifiable) && MultiValue.isMultiValue(value)) {
52+
for (final Object elemInKey : MultiValue.getMultiValueIterable(value)) {
53+
```
54+
55+
## Files Changed
56+
57+
- `engine/src/main/java/com/arcadedb/query/sql/executor/FetchFromIndexStep.java` - fix
58+
- `engine/src/test/java/com/arcadedb/index/InParamIndexExpansionTest.java` - regression tests
59+
60+
## Test Results
61+
62+
New regression test `InParamIndexExpansionTest` (11 test methods):
63+
- `literalInListUsesIndexAndReturnsRows` - passes before and after fix (baseline)
64+
- `namedListParamInWithIndexReturnsRows` - passes before and after fix (`List<Long>` is Iterable)
65+
- `namedPrimitiveLongArrayParamInWithIndexReturnsRows` - **failed before fix, passes after**
66+
- `namedPrimitiveIntArrayParamInWithIndexReturnsRows` - **failed before fix, passes after**
67+
- `namedPrimitiveDoubleArrayParamInWithIndexReturnsRows` - **failed before fix, passes after** (double coerces to INTEGER key)
68+
- `namedObjectArrayParamInWithIndexReturnsRows` - **failed before fix, passes after**
69+
- `positionalListParamInWithIndexReturnsRows` - passes (List is Iterable in embedded mode)
70+
- `namedListParamInExplainUsesIndex` - confirms index is used
71+
- `namedListParamInWithoutIndexReturnsRows` - control: non-indexed path works
72+
- `namedListParamInWithIndexReturnsPartialRows` - partial match works
73+
- `namedListParamInWithIndexReturnsSingleRow` - single-element list works
74+
75+
Broader suite: index (embedded list, list-by-item, composite, map) and SQL query tests pass with
76+
0 failures, 0 errors.
77+
78+
## Review cycles
79+
80+
### Cycle 1 - `b940f325`
81+
82+
- **gemini-code-assist** (1 comment): hoist the `value != null` null check to the front of the
83+
`cartesianProduct` condition. Subsumed by the cycle-1 change below - switching to
84+
`MultiValue.isMultiValue` (internally null-safe) removed the need for any explicit null guard.
85+
- **claude** (review comment): suggested reusing the already-imported `MultiValue` utility instead
86+
of `IterableObjectArray` for consistency with `processInCondition`; add `int[]`/`double[]` test
87+
cases; remove issue-reference comments from test method bodies; remove the docs file.
88+
- Applied: switched to `MultiValue.isMultiValue` + `getMultiValueIterable` (verified safe across
89+
embedded-list, list-by-item, map, and composite index suites); added `int[]` and `double[]`
90+
test cases; removed issue-reference comments and em-dashes from test method bodies.
91+
- Declined: removing this docs file. The `docs/<issue>-<name>.md` tracking doc is an established,
92+
committed convention (38 such files already in the repo, e.g. `docs/4337-*`, `docs/4364-*`) and
93+
is produced by the resolve-issue workflow. The claim that the convention does not exist is not
94+
accurate.
95+
96+
### Cycle 2 - `342fd2b2`
97+
98+
- **claude** (Approved with minor suggestions):
99+
- Noted that switching to `MultiValue` quietly broadens behavior for `Map` parameters (expanded by
100+
`map.values()`), consistent with `processInCondition` but previously not reached. Added a concise
101+
behavioral comment above the condition explaining the multi-value expansion. Did not add a `Map`
102+
test, since that would lock incidental Map-expansion into a tested contract; the fix targets the
103+
array/collection shapes produced by JSON parameter deserialization.
104+
- Declined: trimming the class Javadoc per a cited "never write multi-paragraph docstrings" rule -
105+
that rule is not present in CLAUDE.md, and the Javadoc documents a non-obvious root cause.
106+
- Declined: removing the "Review cycles" section from this doc. 15+ committed tracking docs in the
107+
repo (e.g. `docs/4274-*`, `docs/4331-*`) contain exactly this section; it is a convention.

e2e-ha/pom.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
<parent>
2626
<groupId>com.arcadedb</groupId>
2727
<artifactId>arcadedb-parent</artifactId>
28-
<version>26.6.1-SNAPSHOT</version>
28+
<version>26.6.1</version>
2929
<relativePath>../pom.xml</relativePath>
3030
</parent>
3131

e2e/pom.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
<parent>
2626
<groupId>com.arcadedb</groupId>
2727
<artifactId>arcadedb-parent</artifactId>
28-
<version>26.6.1-SNAPSHOT</version>
28+
<version>26.6.1</version>
2929
<relativePath>../pom.xml</relativePath>
3030
</parent>
3131

engine/pom.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
<parent>
2626
<groupId>com.arcadedb</groupId>
2727
<artifactId>arcadedb-parent</artifactId>
28-
<version>26.6.1-SNAPSHOT</version>
28+
<version>26.6.1</version>
2929
<relativePath>../pom.xml</relativePath>
3030
</parent>
3131

engine/src/main/java/com/arcadedb/GlobalConfiguration.java

Lines changed: 46 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -550,6 +550,11 @@ Enable diagnostic logging during vector graph build progress (heap/off-heap memo
550550
"Root path in the file system where the server is looking for files. By default is the current directory", String.class,
551551
null),
552552

553+
// Default must stay in sync with DefaultLogger.DEFAULT_LOG_DIR so the resolver fallback and the config default agree.
554+
SERVER_LOGS_DIRECTORY("arcadedb.server.logsDirectory", SCOPE.JVM,
555+
"Directory where the server writes log files, referenced as ${arcadedb.server.logsDirectory} in arcadedb-log.properties. Defaults to './log'; set to an absolute writable path for read-only root filesystems.",
556+
String.class, "./log"),
557+
553558
SERVER_DATABASE_DIRECTORY("arcadedb.server.databaseDirectory", SCOPE.JVM, "Directory containing the database", String.class,
554559
"${arcadedb.server.rootPath}/databases"),
555560

@@ -655,10 +660,13 @@ Each idle thread reserves a stack (~512KB-1MB) and Thread metadata in heap, so l
655660

656661
HA_SERVER_LIST("arcadedb.ha.serverList", SCOPE.SERVER,
657662
"""
658-
Servers in the cluster as a list of <hostname/ip-address:raftPort:httpPort[:priority]> items separated by comma. \
663+
Servers in the cluster, comma-separated. Each entry can use either the positional form \
664+
<hostname/ip-address:raftPort:httpPort[:priority[:httpsPort]]> or the more readable object form \
665+
<hostname/ip-address:{raft:2434,http:2480,https:2490,priority:10}> (fields unordered, all optional except raft defaults to the configured Raft port). Both forms may be mixed and prefixed with an optional 'name@'. \
659666
The httpPort is required for replica-to-leader HTTP command forwarding. \
660667
The optional priority (integer, default 0) sets the preferred leader: the node with the highest priority is preferred during elections. \
661-
Example: localhost:2434:2480:10,192.168.0.1:2434:2480:0""",
668+
The optional httpsPort is used for encrypted peer-to-peer transfers (e.g. snapshot download) when 'arcadedb.ssl.enabled' is true; when omitted on a homogeneous cluster it is derived from this node's local HTTPS listening port. \
669+
Examples: localhost:2434:2480:10:2490,192.168.0.1:2434:2480:0:2490 or localhost:{raft:2434,http:2480,https:2490,priority:10},192.168.0.1:{raft:2434,http:2480,https:2490}""",
662670
String.class, ""),
663671

664672
HA_SERVER_ROLE("arcadedb.ha.serverRole", SCOPE.SERVER,
@@ -793,7 +801,7 @@ Set to an absolute path (e.g. /var/lib/arcadedb/raft) to decouple Raft persisten
793801
Path to a file containing the shared secret for inter-node request forwarding authentication. \
794802
Used to keep the secret off the command line (e.g. a Kubernetes Secret mounted on tmpfs). \
795803
Read only when arcadedb.ha.clusterToken is not set; the file content is trimmed of surrounding whitespace.""",
796-
String.class, null),
804+
String.class, ""),
797805

798806
HA_HEALTH_CHECK_INTERVAL("arcadedb.ha.healthCheckInterval", SCOPE.SERVER,
799807
"Interval in milliseconds for the Raft health monitor to check for CLOSED/EXCEPTION state and auto-recover. 0 disables.",
@@ -902,6 +910,22 @@ producing an entry too large for the Raft transport. Kept below the Raft message
902910
"Maximum acceptable gap between the snapshot index and persisted applied index before triggering a snapshot download.",
903911
Long.class, 10L),
904912

913+
HA_STALE_FOLLOWER_LAG_THRESHOLD("arcadedb.ha.staleFollowerLagThreshold", SCOPE.SERVER,
914+
"""
915+
Number of Raft log entries a follower may lag behind the commit index, while NOT actively catching up, before the \
916+
health monitor re-arms a snapshot download from the leader. Guards against a follower that diverged (apply failure) \
917+
and whose snapshot download also failed on a quiet cluster, where no new entry arrives to re-trigger recovery. \
918+
0 (the default) disables stale-follower recovery; the node restart path remains the primary mitigation. \
919+
Enable with a value well below HA_SNAPSHOT_THRESHOLD once you have observed normal catch-up lag for your workload, \
920+
to avoid multiple followers downloading snapshots from the leader at once.""",
921+
Long.class, 0L),
922+
923+
HA_STALE_FOLLOWER_RECOVERY_DURATION_MS("arcadedb.ha.staleFollowerRecoveryDurationMs", SCOPE.SERVER,
924+
"""
925+
How long in milliseconds the lag described by HA_STALE_FOLLOWER_LAG_THRESHOLD must persist continuously \
926+
(across consecutive health-monitor ticks) before recovery is triggered. Avoids acting on transient catch-up lag.""",
927+
Long.class, 60_000L),
928+
905929
HA_SNAPSHOT_MAX_ENTRY_SIZE("arcadedb.ha.snapshotMaxEntrySize", SCOPE.SERVER,
906930
"Maximum uncompressed size in bytes for a single entry in a snapshot ZIP file. Protects against decompression bombs.",
907931
Long.class, 10_737_418_240L),
@@ -925,6 +949,25 @@ producing an entry too large for the Raft transport. Kept below the Raft message
925949
"Rate-limiting interval in milliseconds for DNS re-resolution in the gRPC peer address allowlist filter.",
926950
Long.class, 30_000L),
927951

952+
HA_PEER_ALLOWLIST_STARTUP_GRACE_MS("arcadedb.ha.peerAllowlistStartupGraceMs", SCOPE.SERVER,
953+
"""
954+
Startup grace window in milliseconds during which the gRPC peer allowlist filter fails OPEN (accepts and logs a \
955+
warning) for an inbound address it cannot yet match, as long as it has never resolved every host in \
956+
arcadedb.ha.serverList at least once. This prevents a self-inflicted partition on Kubernetes, where a peer's \
957+
headless-service DNS record is only published once its pod is Ready, so a legitimately-restarting peer connects \
958+
before its own name resolves. Measured from filter creation. Once all peer hosts have resolved at least once, or \
959+
the window elapses, the filter enforces normally. Set to 0 to disable fail-open (strict from the first connection); \
960+
the filter is not an mTLS substitute (see issue #3890), so a bounded fail-open window is the safer default.""",
961+
Long.class, 60_000L),
962+
963+
HA_PEER_ALLOWLIST_STICKY_TTL_MS("arcadedb.ha.peerAllowlistStickyTtlMs", SCOPE.SERVER,
964+
"""
965+
How long in milliseconds the gRPC peer allowlist filter keeps the last successfully-resolved IPs of a peer host \
966+
when a later DNS re-resolution of that host fails. Bridges transient DNS outages and pod-IP churn so a peer that \
967+
resolved moments ago is not evicted from the allowlist by a momentary lookup failure. Set to 0 to disable \
968+
stickiness (drop a host from the allowlist as soon as it stops resolving).""",
969+
Long.class, 300_000L),
970+
928971
// POSTGRES
929972
POSTGRES_PORT("arcadedb.postgres.port", SCOPE.SERVER,
930973
"TCP/IP port number used for incoming connections for Postgres plugin. Default is 5432", Integer.class, 5432),

0 commit comments

Comments
 (0)