Skip to content

Commit ada48ad

Browse files
authored
CNDB-13004 don't fail on too long file name for options (#1576)
When a compaction controller config options are being written or read for a table with long keyspace + table names, FileSystemException happens. This commit fixes that such exception is caught and logged. Thus existing table with long name will continue to function. Tests are added. Also it catches other exceptions.
1 parent 68653f2 commit ada48ad

File tree

5 files changed

+64
-3
lines changed

5 files changed

+64
-3
lines changed

src/java/org/apache/cassandra/db/compaction/unified/AdaptiveController.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@
2424
import javax.annotation.Nullable;
2525

2626
import com.google.common.annotations.VisibleForTesting;
27+
28+
import org.apache.cassandra.io.FSError;
29+
import org.apache.cassandra.utils.JVMStabilityInspector;
2730
import org.slf4j.Logger;
2831
import org.slf4j.LoggerFactory;
2932

@@ -207,6 +210,15 @@ static Controller fromOptions(Environment env,
207210
{
208211
logger.warn("Unable to parse saved options. Using starting value instead:", e);
209212
}
213+
catch (FSError e)
214+
{
215+
logger.warn("Unable to read controller config file. Using starting value instead:", e);
216+
}
217+
catch (Throwable e)
218+
{
219+
logger.warn("Unable to read controller config file. Using starting value instead:", e);
220+
JVMStabilityInspector.inspectThrowable(e);
221+
}
210222

211223
if (scalingParameters == null)
212224
{

src/java/org/apache/cassandra/db/compaction/unified/Controller.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@
3636
import com.google.common.annotations.VisibleForTesting;
3737

3838
import org.apache.cassandra.config.Config;
39+
import org.apache.cassandra.io.FSError;
40+
import org.apache.cassandra.utils.JVMStabilityInspector;
3941
import org.slf4j.Logger;
4042
import org.slf4j.LoggerFactory;
4143

@@ -423,10 +425,15 @@ public static void storeOptions(String keyspaceName, String tableName, int[] sca
423425

424426
logger.debug(String.format("Writing current scaling parameters and flush size to file %s: %s", f.toPath().toString(), jsonObject));
425427
}
426-
catch(IOException e)
428+
catch (IOException | FSError e)
427429
{
428430
logger.warn("Unable to save current scaling parameters and flush size. Current controller configuration will be lost if a node restarts: ", e);
429431
}
432+
catch (Throwable e)
433+
{
434+
logger.warn("Unable to save current scaling parameters and flush size. Current controller configuration will be lost if a node restarts: ", e);
435+
JVMStabilityInspector.inspectThrowable(e);
436+
}
430437
}
431438

432439
public abstract void storeControllerConfig();

src/java/org/apache/cassandra/db/compaction/unified/StaticController.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import org.apache.cassandra.db.compaction.CompactionPick;
2626
import org.apache.cassandra.db.compaction.UnifiedCompactionStrategy;
2727
import org.apache.cassandra.exceptions.ConfigurationException;
28+
import org.apache.cassandra.io.FSError;
2829
import org.apache.cassandra.io.util.File;
2930
import org.apache.cassandra.io.util.FileReader;
3031
import org.apache.cassandra.utils.JVMStabilityInspector;
@@ -151,6 +152,10 @@ static Controller fromOptions(Environment env,
151152
{
152153
logger.warn("Unable to parse saved flush size. Using starting value instead:", e);
153154
}
155+
catch (FSError e)
156+
{
157+
logger.warn("Unable to read controller config file. Using starting value instead:", e);
158+
}
154159
catch (Throwable e)
155160
{
156161
logger.warn("Unable to read controller config file. Using starting value instead:", e);
@@ -235,4 +240,4 @@ public String toString()
235240
printScalingParameters(scalingParameters),
236241
calculator);
237242
}
238-
}
243+
}

test/distributed/org/apache/cassandra/distributed/test/CompactionControllerConfigTest.java

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,28 @@ public void testStoreAndCleanupControllerConfig() throws Throwable
177177
}
178178
}
179179

180+
@Test
181+
public void testStoreLongTableName() throws Throwable
182+
{
183+
try (Cluster cluster = init(Cluster.build(1).start()))
184+
{
185+
cluster.get(1).runOnInstance(() ->
186+
{
187+
CompactionManager.storeControllerConfig();
188+
189+
// try to store controller config for a table with a long name
190+
String keyspaceName = "g38373639353166362d356631322d343864652d393063362d653862616534343165333764_tpch";
191+
String longTableName = "test_create_k8yq1r75bpzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz";
192+
int[] scalingParameters = new int[32];
193+
Arrays.fill(scalingParameters, 5);
194+
AdaptiveController.storeOptions(keyspaceName, longTableName, scalingParameters, 10 << 20);
195+
196+
// verify that the file wasn't created
197+
assert !Controller.getControllerConfigPath(keyspaceName, longTableName).exists();
198+
});
199+
}
200+
}
201+
180202
@Test
181203
public void testVectorControllerConfig() throws Throwable
182204
{
@@ -234,7 +256,6 @@ public void vectorControllerConfig(boolean vectorOverride) throws Throwable
234256
controller2.getTargetSSTableSize());
235257
assertEquals(0, controller2.getScalingParameter(0));
236258
});
237-
238259
}
239260
}
240261
}

test/unit/org/apache/cassandra/db/compaction/unified/AdaptiveControllerTest.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,22 @@ private AdaptiveController makeController(long dataSizeGB, int numShards, long s
9797
tableName);
9898
}
9999

100+
@Test
101+
public void testLongTableNameFromOptions()
102+
{
103+
String longTableName = "test_create_k8yq1r75bpzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz";
104+
when(cfs.getTableName()).thenReturn(longTableName);
105+
106+
Map<String, String> options = new HashMap<>();
107+
options.put(AdaptiveController.THRESHOLD, "0.15");
108+
// Calls fromOptions on long table name, which tries to read options from a file.
109+
// The too long file name should not lead to a failure.
110+
Controller controller = testFromOptions(true, options);
111+
assertTrue(controller instanceof AdaptiveController);
112+
113+
when(cfs.getTableName()).thenReturn(tableName);
114+
}
115+
100116
@Test
101117
public void testFromOptions()
102118
{

0 commit comments

Comments
 (0)