Skip to content

Commit ebbf01e

Browse files
petersmythepetersmythe
committed
Refactor seeder pool config: bean injection instead of SpEL
Address review feedback from pmauduit: - Replace SpEL constructor-arg expressions with bean ref injection (consistent with all other beans in geowebcache-core-context.xml) - Use StringUtils.hasText(), fix test copyright, rename test method - Add tests for ServerConfiguration constructor and env var precedence
1 parent 8e7c624 commit ebbf01e

3 files changed

Lines changed: 137 additions & 43 deletions

File tree

‎geowebcache/core/src/main/java/org/geowebcache/seed/SeederThreadPoolExecutor.java‎

Lines changed: 50 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,31 +19,53 @@
1919
import java.util.concurrent.TimeUnit;
2020
import java.util.logging.Logger;
2121
import org.geotools.util.logging.Logging;
22+
import org.geowebcache.config.ServerConfiguration;
2223
import org.springframework.beans.factory.DisposableBean;
2324
import org.springframework.scheduling.concurrent.CustomizableThreadFactory;
25+
import org.springframework.util.StringUtils;
2426

2527
public class SeederThreadPoolExecutor extends ThreadPoolExecutor implements DisposableBean {
2628

2729
private static final Logger log = Logging.getLogger(SeederThreadPoolExecutor.class.getName());
2830

2931
private static final ThreadFactory tf = new CustomizableThreadFactory("GWC Seeder Thread-");
3032

33+
protected static final int DEFAULT_CORE_POOL_SIZE = 16;
34+
35+
protected static final int DEFAULT_MAX_POOL_SIZE = 32;
36+
3137
/**
3238
* Environment variable / system property name for configuring the core pool size. Looked up from Java system
3339
* properties first, then from OS environment variables. If neither is set or the value is not a valid positive
34-
* integer, the {@code corePoolSize} constructor argument is used as the default.
40+
* integer, the configuration value (or hardcoded default) is used.
3541
*/
3642
public static final String GWC_SEEDER_CORE_POOL_SIZE = "GWC_SEEDER_CORE_POOL_SIZE";
3743

3844
/**
3945
* Environment variable / system property name for configuring the maximum pool size. Looked up from Java system
4046
* properties first, then from OS environment variables. If neither is set or the value is not a valid positive
41-
* integer, the {@code maxPoolSize} constructor argument is used as the default.
47+
* integer, the configuration value (or hardcoded default) is used.
4248
*/
4349
public static final String GWC_SEEDER_MAX_POOL_SIZE = "GWC_SEEDER_MAX_POOL_SIZE";
4450

45-
public SeederThreadPoolExecutor(int corePoolSize, int maxPoolSize) {
46-
this(resolveAndValidateSizes(corePoolSize, maxPoolSize));
51+
/**
52+
* Creates the seeder thread pool, reading pool sizes from the given {@link ServerConfiguration}. The configuration
53+
* values can be overridden by system properties or environment variables.
54+
*
55+
* <p>Precedence: environment variable / system property → ServerConfiguration → hardcoded default (16/32).
56+
*
57+
* @param config the server configuration providing pool size settings from geowebcache.xml
58+
*/
59+
public SeederThreadPoolExecutor(ServerConfiguration config) {
60+
this(configuredCorePoolSize(config), configuredMaxPoolSize(config));
61+
}
62+
63+
/**
64+
* Internal constructor that resolves env var overrides and validates sizes. Subclasses (e.g. GeoServer's
65+
* SeederThreadLocalTransferExecutor) use this to pass in pool sizes read from their own configuration.
66+
*/
67+
protected SeederThreadPoolExecutor(int defaultCore, int defaultMax) {
68+
this(resolvedSizes(defaultCore, defaultMax));
4769
}
4870

4971
private SeederThreadPoolExecutor(int[] sizes) {
@@ -54,10 +76,28 @@ private SeederThreadPoolExecutor(int[] sizes) {
5476
+ getMaximumPoolSize());
5577
}
5678

57-
/**
58-
* Resolves both pool sizes once and validates the core <= max constraint. Returns a two-element array [core, max].
59-
*/
60-
private static int[] resolveAndValidateSizes(int defaultCore, int defaultMax) {
79+
private static int configuredCorePoolSize(ServerConfiguration config) {
80+
if (config != null) {
81+
Integer value = config.getSeederCorePoolSize();
82+
if (value != null && value > 0) {
83+
return value;
84+
}
85+
}
86+
return DEFAULT_CORE_POOL_SIZE;
87+
}
88+
89+
private static int configuredMaxPoolSize(ServerConfiguration config) {
90+
if (config != null) {
91+
Integer value = config.getSeederMaxPoolSize();
92+
if (value != null && value > 0) {
93+
return value;
94+
}
95+
}
96+
return DEFAULT_MAX_POOL_SIZE;
97+
}
98+
99+
/** Resolves both pool sizes applying env var overrides and the core <= max constraint. Returns [core, max]. */
100+
private static int[] resolvedSizes(int defaultCore, int defaultMax) {
61101
int core = resolvePoolSize(GWC_SEEDER_CORE_POOL_SIZE, defaultCore);
62102
int max = resolvePoolSize(GWC_SEEDER_MAX_POOL_SIZE, defaultMax);
63103
if (core > max) {
@@ -82,10 +122,10 @@ private static int[] resolveAndValidateSizes(int defaultCore, int defaultMax) {
82122
*/
83123
static int resolvePoolSize(String propertyName, int defaultValue) {
84124
String value = System.getProperty(propertyName);
85-
if (value == null || value.isBlank()) {
125+
if (!StringUtils.hasText(value)) {
86126
value = System.getenv(propertyName);
87127
}
88-
if (value != null && !value.isBlank()) {
128+
if (StringUtils.hasText(value)) {
89129
try {
90130
int parsed = Integer.parseInt(value.trim());
91131
if (parsed > 0) {

‎geowebcache/core/src/test/java/org/geowebcache/seed/SeederThreadPoolExecutorTest.java‎

Lines changed: 86 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,16 @@
88
*
99
* <p>You should have received a copy of the GNU Lesser General Public License along with this program. If not, see
1010
* <http://www.gnu.org/licenses/>.
11+
*
12+
* <p>Copyright 2026
1113
*/
1214
package org.geowebcache.seed;
1315

1416
import static org.junit.Assert.assertEquals;
17+
import static org.mockito.Mockito.mock;
18+
import static org.mockito.Mockito.when;
1519

20+
import org.geowebcache.config.ServerConfiguration;
1621
import org.geowebcache.util.PropertyRule;
1722
import org.junit.Rule;
1823
import org.junit.Test;
@@ -27,11 +32,14 @@ public class SeederThreadPoolExecutorTest {
2732

2833
@Test
2934
public void testDefaultValues() {
30-
// No system property set, should use the constructor-provided defaults
3135
corePoolSizeProp.setValue(null);
3236
maxPoolSizeProp.setValue(null);
3337

34-
SeederThreadPoolExecutor executor = new SeederThreadPoolExecutor(16, 32);
38+
ServerConfiguration config = mock(ServerConfiguration.class);
39+
when(config.getSeederCorePoolSize()).thenReturn(null);
40+
when(config.getSeederMaxPoolSize()).thenReturn(null);
41+
42+
SeederThreadPoolExecutor executor = new SeederThreadPoolExecutor(config);
3543
try {
3644
assertEquals(16, executor.getCorePoolSize());
3745
assertEquals(32, executor.getMaximumPoolSize());
@@ -41,71 +49,108 @@ public void testDefaultValues() {
4149
}
4250

4351
@Test
44-
public void testSystemPropertyOverride() {
45-
corePoolSizeProp.setValue("24");
46-
maxPoolSizeProp.setValue("64");
52+
public void testConfiguredValues() {
53+
corePoolSizeProp.setValue(null);
54+
maxPoolSizeProp.setValue(null);
55+
56+
ServerConfiguration config = mock(ServerConfiguration.class);
57+
when(config.getSeederCorePoolSize()).thenReturn(24);
58+
when(config.getSeederMaxPoolSize()).thenReturn(48);
4759

48-
SeederThreadPoolExecutor executor = new SeederThreadPoolExecutor(16, 32);
60+
SeederThreadPoolExecutor executor = new SeederThreadPoolExecutor(config);
4961
try {
5062
assertEquals(24, executor.getCorePoolSize());
51-
assertEquals(64, executor.getMaximumPoolSize());
63+
assertEquals(48, executor.getMaximumPoolSize());
5264
} finally {
5365
executor.shutdownNow();
5466
}
5567
}
5668

5769
@Test
58-
public void testInvalidValueFallsBackToDefault() {
70+
public void testSystemPropertyOverridesConfig() {
71+
corePoolSizeProp.setValue("100");
72+
maxPoolSizeProp.setValue("200");
73+
74+
ServerConfiguration config = mock(ServerConfiguration.class);
75+
when(config.getSeederCorePoolSize()).thenReturn(24);
76+
when(config.getSeederMaxPoolSize()).thenReturn(48);
77+
78+
SeederThreadPoolExecutor executor = new SeederThreadPoolExecutor(config);
79+
try {
80+
assertEquals(100, executor.getCorePoolSize());
81+
assertEquals(200, executor.getMaximumPoolSize());
82+
} finally {
83+
executor.shutdownNow();
84+
}
85+
}
86+
87+
@Test
88+
public void testInvalidValueFallsBackToConfig() {
5989
corePoolSizeProp.setValue("invalid");
6090
maxPoolSizeProp.setValue("notanumber");
6191

62-
SeederThreadPoolExecutor executor = new SeederThreadPoolExecutor(16, 32);
92+
ServerConfiguration config = mock(ServerConfiguration.class);
93+
when(config.getSeederCorePoolSize()).thenReturn(24);
94+
when(config.getSeederMaxPoolSize()).thenReturn(48);
95+
96+
SeederThreadPoolExecutor executor = new SeederThreadPoolExecutor(config);
6397
try {
64-
assertEquals(16, executor.getCorePoolSize());
65-
assertEquals(32, executor.getMaximumPoolSize());
98+
assertEquals(24, executor.getCorePoolSize());
99+
assertEquals(48, executor.getMaximumPoolSize());
66100
} finally {
67101
executor.shutdownNow();
68102
}
69103
}
70104

71105
@Test
72-
public void testNegativeValueFallsBackToDefault() {
106+
public void testNonPositiveValueFallsBackToConfig() {
73107
corePoolSizeProp.setValue("-5");
74108
maxPoolSizeProp.setValue("0");
75109

76-
SeederThreadPoolExecutor executor = new SeederThreadPoolExecutor(16, 32);
110+
ServerConfiguration config = mock(ServerConfiguration.class);
111+
when(config.getSeederCorePoolSize()).thenReturn(24);
112+
when(config.getSeederMaxPoolSize()).thenReturn(48);
113+
114+
SeederThreadPoolExecutor executor = new SeederThreadPoolExecutor(config);
77115
try {
78-
assertEquals(16, executor.getCorePoolSize());
79-
assertEquals(32, executor.getMaximumPoolSize());
116+
assertEquals(24, executor.getCorePoolSize());
117+
assertEquals(48, executor.getMaximumPoolSize());
80118
} finally {
81119
executor.shutdownNow();
82120
}
83121
}
84122

85123
@Test
86124
public void testPartialOverride() {
87-
// Only override core pool size, leave max at default
88125
corePoolSizeProp.setValue("20");
89126
maxPoolSizeProp.setValue(null);
90127

91-
SeederThreadPoolExecutor executor = new SeederThreadPoolExecutor(16, 32);
128+
ServerConfiguration config = mock(ServerConfiguration.class);
129+
when(config.getSeederCorePoolSize()).thenReturn(null);
130+
when(config.getSeederMaxPoolSize()).thenReturn(48);
131+
132+
SeederThreadPoolExecutor executor = new SeederThreadPoolExecutor(config);
92133
try {
93134
assertEquals(20, executor.getCorePoolSize());
94-
assertEquals(32, executor.getMaximumPoolSize());
135+
assertEquals(48, executor.getMaximumPoolSize());
95136
} finally {
96137
executor.shutdownNow();
97138
}
98139
}
99140

100141
@Test
101-
public void testWhitespaceValueFallsBackToDefault() {
142+
public void testWhitespaceValueFallsBackToConfig() {
102143
corePoolSizeProp.setValue(" ");
103144
maxPoolSizeProp.setValue("");
104145

105-
SeederThreadPoolExecutor executor = new SeederThreadPoolExecutor(16, 32);
146+
ServerConfiguration config = mock(ServerConfiguration.class);
147+
when(config.getSeederCorePoolSize()).thenReturn(24);
148+
when(config.getSeederMaxPoolSize()).thenReturn(48);
149+
150+
SeederThreadPoolExecutor executor = new SeederThreadPoolExecutor(config);
106151
try {
107-
assertEquals(16, executor.getCorePoolSize());
108-
assertEquals(32, executor.getMaximumPoolSize());
152+
assertEquals(24, executor.getCorePoolSize());
153+
assertEquals(48, executor.getMaximumPoolSize());
109154
} finally {
110155
executor.shutdownNow();
111156
}
@@ -116,7 +161,11 @@ public void testValueWithWhitespaceIsTrimmed() {
116161
corePoolSizeProp.setValue(" 24 ");
117162
maxPoolSizeProp.setValue(" 48 ");
118163

119-
SeederThreadPoolExecutor executor = new SeederThreadPoolExecutor(16, 32);
164+
ServerConfiguration config = mock(ServerConfiguration.class);
165+
when(config.getSeederCorePoolSize()).thenReturn(null);
166+
when(config.getSeederMaxPoolSize()).thenReturn(null);
167+
168+
SeederThreadPoolExecutor executor = new SeederThreadPoolExecutor(config);
120169
try {
121170
assertEquals(24, executor.getCorePoolSize());
122171
assertEquals(48, executor.getMaximumPoolSize());
@@ -127,11 +176,14 @@ public void testValueWithWhitespaceIsTrimmed() {
127176

128177
@Test
129178
public void testCorePoolSizeExceedsMaxPoolSizeAdjustsMax() {
130-
// Set core higher than max — max should be adjusted upward to match core
131179
corePoolSizeProp.setValue("64");
132180
maxPoolSizeProp.setValue("32");
133181

134-
SeederThreadPoolExecutor executor = new SeederThreadPoolExecutor(16, 32);
182+
ServerConfiguration config = mock(ServerConfiguration.class);
183+
when(config.getSeederCorePoolSize()).thenReturn(null);
184+
when(config.getSeederMaxPoolSize()).thenReturn(null);
185+
186+
SeederThreadPoolExecutor executor = new SeederThreadPoolExecutor(config);
135187
try {
136188
assertEquals(64, executor.getCorePoolSize());
137189
assertEquals(64, executor.getMaximumPoolSize());
@@ -141,15 +193,18 @@ public void testCorePoolSizeExceedsMaxPoolSizeAdjustsMax() {
141193
}
142194

143195
@Test
144-
public void testCorePoolSizeOverrideExceedsDefaultMax() {
145-
// Only override core to be higher than the default max — max should adjust
146-
corePoolSizeProp.setValue("48");
196+
public void testConfigCoreExceedsConfigMaxAdjustsMax() {
197+
corePoolSizeProp.setValue(null);
147198
maxPoolSizeProp.setValue(null);
148199

149-
SeederThreadPoolExecutor executor = new SeederThreadPoolExecutor(16, 32);
200+
ServerConfiguration config = mock(ServerConfiguration.class);
201+
when(config.getSeederCorePoolSize()).thenReturn(64);
202+
when(config.getSeederMaxPoolSize()).thenReturn(32);
203+
204+
SeederThreadPoolExecutor executor = new SeederThreadPoolExecutor(config);
150205
try {
151-
assertEquals(48, executor.getCorePoolSize());
152-
assertEquals(48, executor.getMaximumPoolSize());
206+
assertEquals(64, executor.getCorePoolSize());
207+
assertEquals(64, executor.getMaximumPoolSize());
153208
} finally {
154209
executor.shutdownNow();
155210
}

‎geowebcache/web/src/main/webapp/WEB-INF/geowebcache-core-context.xml‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -180,8 +180,7 @@
180180
<!-- Thread pool for seeding -->
181181
<bean id="gwcSeederThreadPoolExec"
182182
class="org.geowebcache.seed.SeederThreadPoolExecutor">
183-
<constructor-arg value="#{gwcXmlConfig.seederCorePoolSize != null ? gwcXmlConfig.seederCorePoolSize : 16}"/><!-- Size of core pool -->
184-
<constructor-arg value="#{gwcXmlConfig.seederMaxPoolSize != null ? gwcXmlConfig.seederMaxPoolSize : 32}"/><!-- Maximum size of pool -->
183+
<constructor-arg ref="gwcXmlConfig"/>
185184
</bean>
186185

187186
<!-- Breeder (the one that seeds) -->

0 commit comments

Comments
 (0)