Skip to content

Commit a3deddd

Browse files
k-rusdjatnieks
authored andcommitted
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 3af7bcb commit a3deddd

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
@@ -23,6 +23,9 @@
2323
import javax.annotation.Nullable;
2424

2525
import com.google.common.annotations.VisibleForTesting;
26+
27+
import org.apache.cassandra.io.FSError;
28+
import org.apache.cassandra.utils.JVMStabilityInspector;
2629
import org.slf4j.Logger;
2730
import org.slf4j.LoggerFactory;
2831

@@ -210,6 +213,15 @@ static Controller fromOptions(Environment env,
210213
{
211214
logger.warn("Unable to parse saved options. Using starting value instead:", e);
212215
}
216+
catch (FSError e)
217+
{
218+
logger.warn("Unable to read controller config file. Using starting value instead:", e);
219+
}
220+
catch (Throwable e)
221+
{
222+
logger.warn("Unable to read controller config file. Using starting value instead:", e);
223+
JVMStabilityInspector.inspectThrowable(e);
224+
}
213225

214226
if (scalingParameters == null)
215227
{

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,13 +48,15 @@
4848
import org.apache.cassandra.db.compaction.UnifiedCompactionStrategy;
4949
import org.apache.cassandra.db.marshal.VectorType;
5050
import org.apache.cassandra.exceptions.ConfigurationException;
51+
import org.apache.cassandra.io.FSError;
5152
import org.apache.cassandra.io.util.File;
5253
import org.apache.cassandra.io.util.FileWriter;
5354
import org.apache.cassandra.metrics.DefaultNameFactory;
5455
import org.apache.cassandra.metrics.MetricNameFactory;
5556
import org.apache.cassandra.schema.SchemaConstants;
5657
import org.apache.cassandra.schema.TableMetadata;
5758
import org.apache.cassandra.utils.FBUtilities;
59+
import org.apache.cassandra.utils.JVMStabilityInspector;
5860
import org.apache.cassandra.utils.MonotonicClock;
5961
import org.apache.cassandra.utils.Overlaps;
6062
import org.json.simple.JSONArray;
@@ -426,10 +428,15 @@ public static void storeOptions(String keyspaceName, String tableName, int[] sca
426428

427429
logger.debug(String.format("Writing current scaling parameters and flush size to file %s: %s", f.toPath().toString(), jsonObject));
428430
}
429-
catch(IOException e)
431+
catch (IOException | FSError e)
430432
{
431433
logger.warn("Unable to save current scaling parameters and flush size. Current controller configuration will be lost if a node restarts: ", e);
432434
}
435+
catch (Throwable e)
436+
{
437+
logger.warn("Unable to save current scaling parameters and flush size. Current controller configuration will be lost if a node restarts: ", e);
438+
JVMStabilityInspector.inspectThrowable(e);
439+
}
433440
}
434441

435442
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;
@@ -154,6 +155,10 @@ static Controller fromOptions(Environment env,
154155
{
155156
logger.warn("Unable to parse saved flush size. Using starting value instead:", e);
156157
}
158+
catch (FSError e)
159+
{
160+
logger.warn("Unable to read controller config file. Using starting value instead:", e);
161+
}
157162
catch (Throwable e)
158163
{
159164
logger.warn("Unable to read controller config file. Using starting value instead:", e);
@@ -238,4 +243,4 @@ public String toString()
238243
printScalingParameters(scalingParameters),
239244
calculator);
240245
}
241-
}
246+
}

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

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

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

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)