Skip to content

Commit b8bf8af

Browse files
committed
withMemoryLimitMB -> withArrowMemoryLimitMB
1 parent 0e0fa45 commit b8bf8af

3 files changed

Lines changed: 53 additions & 17 deletions

File tree

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ The `SpiceClient` uses an Arrow `RootAllocator` for managing off-heap memory. By
128128

129129
```java
130130
SpiceClient client = SpiceClient.builder()
131-
.withMemoryLimitMB(1024) // 1GB limit
131+
.withArrowMemoryLimitMB(1024) // 1GB limit
132132
.build();
133133
```
134134

src/main/java/ai/spice/SpiceClientBuilder.java

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -143,19 +143,26 @@ public SpiceClientBuilder withMaxRetries(int maxRetries) {
143143
/**
144144
* Sets the memory limit for Apache Arrow allocator in megabytes.
145145
* This controls the maximum amount of off-heap memory that can be allocated
146-
* for Arrow Flight operations. If not set, the allocator will use all available memory.
146+
* for Arrow Flight operations. If not set, the allocator will use all available
147+
* memory.
147148
*
148-
* @param memoryLimitMB Maximum memory limit in megabytes. Default is all available memory.
149+
* @param memoryLimitMB Maximum memory limit in megabytes. Default is all
150+
* available memory.
149151
* Must be positive.
150152
* @return The current instance of SpiceClientBuilder for method chaining.
151153
* @throws IllegalArgumentException if memoryLimitMB is not positive
152154
*
153155
* @see org.apache.arrow.memory.RootAllocator
154156
*/
155-
public SpiceClientBuilder withMemoryLimitMB(long memoryLimitMB) {
157+
public SpiceClientBuilder withArrowMemoryLimitMB(long memoryLimitMB) {
156158
if (memoryLimitMB <= 0) {
157159
throw new IllegalArgumentException("Memory limit must be positive, got: " + memoryLimitMB + " MB");
158160
}
161+
if (memoryLimitMB > Long.MAX_VALUE / 1024L / 1024L) {
162+
throw new IllegalArgumentException(
163+
"Memory limit is too large: " + memoryLimitMB + " MB");
164+
}
165+
159166
this.memoryLimitMB = memoryLimitMB;
160167
return this;
161168
}

src/test/java/ai/spice/MemoryConfigurationTest.java

Lines changed: 42 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11,41 +11,70 @@ public void testMemoryLimitMBConfiguration() throws Exception {
1111
try {
1212
// Test that memory limit in MB is properly configured
1313
SpiceClient client = SpiceClient.builder()
14-
.withMemoryLimitMB(128) // 128 MB
15-
.build();
16-
14+
.withArrowMemoryLimitMB(128) // 128 MB
15+
.build();
16+
1717
// If we reach here without exception, the configuration worked
1818
assertTrue("Memory configuration should not throw exception", true);
19-
19+
2020
client.close();
2121
} catch (Exception e) {
22-
// We expect some exceptions due to no local Spice instance,
23-
// but not IllegalArgumentException from memory configuration
24-
assertFalse("Should not throw IllegalArgumentException for valid memory config",
25-
e instanceof IllegalArgumentException);
22+
fail("Should not throw exception: " + e.getMessage());
2623
}
2724
}
2825

2926
public void testInvalidMemoryLimitMB() throws Exception {
3027
try {
3128
SpiceClient.builder()
32-
.withMemoryLimitMB(0) // Invalid: must be positive
33-
.build();
29+
.withArrowMemoryLimitMB(0) // Invalid: must be positive
30+
.build();
3431
fail("Should throw IllegalArgumentException for zero memory limit");
3532
} catch (IllegalArgumentException e) {
3633
// Expected exception
37-
assertTrue("Should throw IllegalArgumentException for zero memory limit", true);
34+
assertTrue("Should throw IllegalArgumentException for zero memory limit", true);
3835
}
3936
}
4037

4138
public void testNegativeMemoryLimitMB() throws Exception {
4239
try {
4340
SpiceClient.builder()
44-
.withMemoryLimitMB(-100) // Invalid: negative
45-
.build();
41+
.withArrowMemoryLimitMB(-100) // Invalid: negative
42+
.build();
4643
fail("Should throw IllegalArgumentException for negative memory limit");
4744
} catch (IllegalArgumentException e) {
4845
assertTrue("Should throw IllegalArgumentException for negative memory limit", true);
4946
}
5047
}
48+
49+
public void testOverflowProtection() throws Exception {
50+
// Test overflow protection - values that would cause overflow when converted to
51+
// bytes
52+
long maxSafeMB = Long.MAX_VALUE / (1024L * 1024L);
53+
long overflowValue = maxSafeMB + 1;
54+
55+
try {
56+
SpiceClient.builder()
57+
.withArrowMemoryLimitMB(overflowValue) // Would cause overflow
58+
.build();
59+
fail("Should throw IllegalArgumentException for overflow-causing memory limit");
60+
} catch (IllegalArgumentException e) {
61+
assertTrue("Should throw IllegalArgumentException for overflow protection", true);
62+
}
63+
}
64+
65+
public void testMaxSafeMemoryLimit() throws Exception {
66+
// Test that the maximum safe value works without throwing
67+
long maxSafeMB = Long.MAX_VALUE / (1024L * 1024L);
68+
69+
try {
70+
SpiceClient client = SpiceClient.builder()
71+
.withArrowMemoryLimitMB(maxSafeMB) // Maximum safe value
72+
.build();
73+
74+
assertTrue("Maximum safe memory limit should work", true);
75+
client.close();
76+
} catch (Exception e) {
77+
fail("Should not throw exception: " + e.getMessage());
78+
}
79+
}
5180
}

0 commit comments

Comments
 (0)