[jira] [Commented] (CASSANDRA-4049) Add generic way of adding SSTable components required custom compaction strategy
[
https://issues.apache.org/jira/browse/CASSANDRA-4049?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13486900#comment-13486900
]
Piotr Kołaczkowski commented on CASSANDRA-4049:
---
Fixed the patch so it doesn't break tests - there is no need to create TOC if
the SSTable doesn't exist at all.
diff between the previous patch:
{noformat}
119c119
index 9a29066..b000d2f 100644
---
index 9a29066..e84f561 100644
157c157
@@ -175,36 +178,51 @@ public abstract class SSTable
---
@@ -175,36 +178,49 @@ public abstract class SSTable
184,185d183
+if (components.isEmpty())
+return components;
223c221
@@ -253,4 +271,64 @@ public abstract class SSTable
---
@@ -253,4 +269,64 @@ public abstract class SSTable
{noformat}
Add generic way of adding SSTable components required custom compaction
strategy
Key: CASSANDRA-4049
URL: https://issues.apache.org/jira/browse/CASSANDRA-4049
Project: Cassandra
Issue Type: New Feature
Components: Core
Reporter: Piotr Kołaczkowski
Assignee: Piotr Kołaczkowski
Priority: Minor
Labels: compaction
Fix For: 1.2.0
Attachments: 4049-v3.txt, 4049-v4.txt, 4049-v5.txt,
pluggable_custom_components-1.1.5-2.patch,
pluggable_custom_components-1.1.5.patch
CFS compaction strategy coming up in the next DSE release needs to store some
important information in Tombstones.db and RemovedKeys.db files, one per
sstable. However, currently Cassandra issues warnings when these files are
found in the data directory. Additionally, when switched to
SizeTieredCompactionStrategy, the files are left in the data directory after
compaction.
The attached patch adds new components to the Component class so Cassandra
knows about those files.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CASSANDRA-4049) Add generic way of adding SSTable components required custom compaction strategy
[ https://issues.apache.org/jira/browse/CASSANDRA-4049?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13484003#comment-13484003 ] Piotr Kołaczkowski commented on CASSANDRA-4049: --- Ok, I take a look at it. Add generic way of adding SSTable components required custom compaction strategy Key: CASSANDRA-4049 URL: https://issues.apache.org/jira/browse/CASSANDRA-4049 Project: Cassandra Issue Type: New Feature Components: Core Reporter: Piotr Kołaczkowski Assignee: Piotr Kołaczkowski Priority: Minor Labels: compaction Fix For: 1.2.0 beta 2 Attachments: 4049-v3.txt, 4049-v4.txt, pluggable_custom_components-1.1.5-2.patch, pluggable_custom_components-1.1.5.patch CFS compaction strategy coming up in the next DSE release needs to store some important information in Tombstones.db and RemovedKeys.db files, one per sstable. However, currently Cassandra issues warnings when these files are found in the data directory. Additionally, when switched to SizeTieredCompactionStrategy, the files are left in the data directory after compaction. The attached patch adds new components to the Component class so Cassandra knows about those files. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CASSANDRA-4049) Add generic way of adding SSTable components required custom compaction strategy
[
https://issues.apache.org/jira/browse/CASSANDRA-4049?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13452835#comment-13452835
]
Piotr Kołaczkowski commented on CASSANDRA-4049:
---
Sorry for late reply, been busy fixing things in dse.
{quote}
{noformat}
catch (Exception e)
{
if (!snapshots.equals(name) !backups.equals(name)
!name.contains(.json))
logger.warn(Invalid file '{}' in data directory {}., name,
dir);
return null;
}
{noformat}
What was the reasoning behind this? Not saying it's wrong to remove it, but I
want to make sure we understand what it was supposed to do, before deciding we
don't need it.
{quote}
This check was there before. Actually because this check was firing out
warnings, we created this ticket ;)
-
{noformat}
catch (IOException e)
{
SetComponent components =
Sets.newHashSetWithExpectedSize(Component.TYPES.size());
for (Component.Type componentType : Component.TYPES)
{
Component component = new Component(componentType);
if (new File(desc.filenameFor(component)).exists())
components.add(component);
}
saveTOC(desc, components);
return components;
}
{noformat}
This one is for backwards compatibility. If we find an SSTable without a TOC
component (from previous version of C*), we just do what we always did - loop
through all C* components.
{quote}
Use FileUtils.closeQuietly{quote}
Oh, yeah. I was looking for IOUtils.closeQuietly, and couldn't find it. Thanks,
that is what I needed!
{quote}
But probably simpler to just use Guava's Files.readLines{quote}
Ok, I fix it.
{quote}
Do we not know what components are necessary at construction time? Would
strongly prefer an immutable set. Adding extra parameters to SSTW to do this is
fine.{quote}
We do, but there is a chicken-and-egg problem here. CompactionStrategy knows.
But CompactionStrategy needs a set of SSTables to be created. And to create
SSTable readers you need to know the components. That is why I decided for a
TOC component, that allows for reading the list of components at SSTable
construction time.
The workflow of creating a new SSTable is currently as follows:
1. memtable is flushed to disk, with C* only components
2. compaction strategy is notified that a new sstable was created and gets an
SSTableReader (with only default components)
3. compaction strategy adds its custom components to it; in order to do it, it
has to read some components of SSTable (e.g. access the index or data file)
In order to make SSTableReader immutable, we had to ask currently installed
compaction strategy for custom component list somewhere in the middle of this
process, before creating SSTableReader. That is slightly more complex than what
we have now (have to change the CS interface), but retaining full immutability
is probably worth it.
{noformat}
public synchronized void addCustomComponent(Component component){noformat}
You are right that synchronized is wrong here.
Thanks for great suggestions [~jbellis]. I look into improving my patch as soon
as I'm done with the tickets I've got in the waiting queue for DSE 3.0.
Add generic way of adding SSTable components required custom compaction
strategy
Key: CASSANDRA-4049
URL: https://issues.apache.org/jira/browse/CASSANDRA-4049
Project: Cassandra
Issue Type: New Feature
Components: Core
Reporter: Piotr Kołaczkowski
Assignee: Piotr Kołaczkowski
Priority: Minor
Labels: compaction
Fix For: 1.1.6
Attachments: pluggable_custom_components-1.1.4.patch
CFS compaction strategy coming up in the next DSE release needs to store some
important information in Tombstones.db and RemovedKeys.db files, one per
sstable. However, currently Cassandra issues warnings when these files are
found in the data directory. Additionally, when switched to
SizeTieredCompactionStrategy, the files are left in the data directory after
compaction.
The attached patch adds new components to the Component class so Cassandra
knows about those files.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CASSANDRA-4049) Add generic way of adding SSTable components required custom compaction strategy
[
https://issues.apache.org/jira/browse/CASSANDRA-4049?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13445973#comment-13445973
]
Jonathan Ellis commented on CASSANDRA-4049:
---
{noformat}
. catch (Exception e)
{
if (!snapshots.equals(name) !backups.equals(name)
!name.contains(.json))
logger.warn(Invalid file '{}' in data directory {}., name,
dir);
return null;
}
{noformat}
What was the reasoning behind this? Not saying it's wrong to remove it, but I
want to make sure we understand what it was supposed to do, before deciding we
don't need it.
similarly,
{noformat}
. catch (IOException e)
{
SetComponent components =
Sets.newHashSetWithExpectedSize(Component.TYPES.size());
for (Component.Type componentType : Component.TYPES)
{
Component component = new Component(componentType);
if (new File(desc.filenameFor(component)).exists())
components.add(component);
}
saveTOC(desc, components);
return components;
}
{noformat}
what are we trying to recover from here? ISTM that at the least we should log
an error instead of silently skipping over damage.
{noformat}
try
{
if (r != null)
r.close();
}
catch (IOException e)
{
}
{noformat}
Use FileUtils.closeQuietly. But probably simpler to just use Guava's
Files.readLines.
{noformat}
. catch (IOException e)
{
logger.error(Error saving TOC for sstable: + descriptor, e);
}
{noformat}
Looks to me like we should throw IOError since this Shouldn't Happen.
{noformat}
public synchronized void addCustomComponent(Component component)
{noformat}
Do we not know what components are necessary at construction time? Would
strongly prefer an immutable set. Adding extra parameters to SSTW to do this
is fine.
If we must make it mutable, prefer e.g. CopyOnWriteArraySet to using
synchronized. (Otherwise synchronizing write access is not enough for safety,
must also synchronize reads.)
Add generic way of adding SSTable components required custom compaction
strategy
Key: CASSANDRA-4049
URL: https://issues.apache.org/jira/browse/CASSANDRA-4049
Project: Cassandra
Issue Type: New Feature
Components: Core
Reporter: Piotr Kołaczkowski
Assignee: Piotr Kołaczkowski
Priority: Minor
Labels: compaction
Fix For: 1.1.5
Attachments: pluggable_custom_components-1.1.4.patch
CFS compaction strategy coming up in the next DSE release needs to store some
important information in Tombstones.db and RemovedKeys.db files, one per
sstable. However, currently Cassandra issues warnings when these files are
found in the data directory. Additionally, when switched to
SizeTieredCompactionStrategy, the files are left in the data directory after
compaction.
The attached patch adds new components to the Component class so Cassandra
knows about those files.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CASSANDRA-4049) Add generic way of adding SSTable components required custom compaction strategy
[ https://issues.apache.org/jira/browse/CASSANDRA-4049?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13443899#comment-13443899 ] Piotr Kołaczkowski commented on CASSANDRA-4049: --- Ok, rebased. Actually only line numbers changed, there was no conflict. Add generic way of adding SSTable components required custom compaction strategy Key: CASSANDRA-4049 URL: https://issues.apache.org/jira/browse/CASSANDRA-4049 Project: Cassandra Issue Type: New Feature Components: Core Reporter: Piotr Kołaczkowski Assignee: Piotr Kołaczkowski Priority: Minor Labels: compaction Fix For: 1.1.5 Attachments: pluggable_custom_components-1.1.4.patch CFS compaction strategy coming up in the next DSE release needs to store some important information in Tombstones.db and RemovedKeys.db files, one per sstable. However, currently Cassandra issues warnings when these files are found in the data directory. Additionally, when switched to SizeTieredCompactionStrategy, the files are left in the data directory after compaction. The attached patch adds new components to the Component class so Cassandra knows about those files. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CASSANDRA-4049) Add generic way of adding SSTable components required custom compaction strategy
[ https://issues.apache.org/jira/browse/CASSANDRA-4049?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13423513#comment-13423513 ] Jonathan Ellis commented on CASSANDRA-4049: --- Sorry for the delay, would you mind rebasing to current trunk? Add generic way of adding SSTable components required custom compaction strategy Key: CASSANDRA-4049 URL: https://issues.apache.org/jira/browse/CASSANDRA-4049 Project: Cassandra Issue Type: New Feature Components: Core Reporter: Piotr Kołaczkowski Assignee: Piotr Kołaczkowski Priority: Minor Labels: compaction Fix For: 1.1.4 Attachments: pluggable_custom_components.patch CFS compaction strategy coming up in the next DSE release needs to store some important information in Tombstones.db and RemovedKeys.db files, one per sstable. However, currently Cassandra issues warnings when these files are found in the data directory. Additionally, when switched to SizeTieredCompactionStrategy, the files are left in the data directory after compaction. The attached patch adds new components to the Component class so Cassandra knows about those files. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CASSANDRA-4049) Add generic way of adding SSTable components required custom compaction strategy
[ https://issues.apache.org/jira/browse/CASSANDRA-4049?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13397436#comment-13397436 ] Piotr Kołaczkowski commented on CASSANDRA-4049: --- Hmm, I'm also not sure how to do that, but giving them custom names is important. What if we want to provide two compaction strategies using custom components instead of one? What if user wants to switch from one strategy to another one? We'd have to be very careful that they don't use the same custom component for storing different things. So it would be also quite fragile. Add generic way of adding SSTable components required custom compaction strategy Key: CASSANDRA-4049 URL: https://issues.apache.org/jira/browse/CASSANDRA-4049 Project: Cassandra Issue Type: New Feature Components: Core Reporter: Piotr Kołaczkowski Assignee: Piotr Kołaczkowski Priority: Minor Labels: compaction Fix For: 1.1.2 Attachments: compaction_strategy_cleanup.patch, component_patch.diff CFS compaction strategy coming up in the next DSE release needs to store some important information in Tombstones.db and RemovedKeys.db files, one per sstable. However, currently Cassandra issues warnings when these files are found in the data directory. Additionally, when switched to SizeTieredCompactionStrategy, the files are left in the data directory after compaction. The attached patch adds new components to the Component class so Cassandra knows about those files. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CASSANDRA-4049) Add generic way of adding SSTable components required custom compaction strategy
[ https://issues.apache.org/jira/browse/CASSANDRA-4049?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13397442#comment-13397442 ] Piotr Kołaczkowski commented on CASSANDRA-4049: --- If Component.Type was not a static enum, we could provide an API to register new component types. So DSE could invoke that API on startup, register all components required for its custom compaction strategies, and then C* would know about all the components, and we could use components with whatever names we like. If you like the idea, I can do it. Add generic way of adding SSTable components required custom compaction strategy Key: CASSANDRA-4049 URL: https://issues.apache.org/jira/browse/CASSANDRA-4049 Project: Cassandra Issue Type: New Feature Components: Core Reporter: Piotr Kołaczkowski Assignee: Piotr Kołaczkowski Priority: Minor Labels: compaction Fix For: 1.1.2 Attachments: compaction_strategy_cleanup.patch, component_patch.diff CFS compaction strategy coming up in the next DSE release needs to store some important information in Tombstones.db and RemovedKeys.db files, one per sstable. However, currently Cassandra issues warnings when these files are found in the data directory. Additionally, when switched to SizeTieredCompactionStrategy, the files are left in the data directory after compaction. The attached patch adds new components to the Component class so Cassandra knows about those files. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CASSANDRA-4049) Add generic way of adding SSTable components required custom compaction strategy
[ https://issues.apache.org/jira/browse/CASSANDRA-4049?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13396119#comment-13396119 ] Piotr Kołaczkowski commented on CASSANDRA-4049: --- Custom1..Custom5 types would be ok, but we'd like they can at least get meaningful names in the DSE code and in the data-directory. It can pretty fast bite us, if there are files named sblocks-custom1.dat, sblocks-custom2.dat, etc - the name doesn't tell anything what is inside. Add generic way of adding SSTable components required custom compaction strategy Key: CASSANDRA-4049 URL: https://issues.apache.org/jira/browse/CASSANDRA-4049 Project: Cassandra Issue Type: New Feature Components: Core Reporter: Piotr Kołaczkowski Assignee: Piotr Kołaczkowski Priority: Minor Labels: compaction Fix For: 1.1.2 Attachments: compaction_strategy_cleanup.patch, component_patch.diff CFS compaction strategy coming up in the next DSE release needs to store some important information in Tombstones.db and RemovedKeys.db files, one per sstable. However, currently Cassandra issues warnings when these files are found in the data directory. Additionally, when switched to SizeTieredCompactionStrategy, the files are left in the data directory after compaction. The attached patch adds new components to the Component class so Cassandra knows about those files. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CASSANDRA-4049) Add generic way of adding SSTable components required custom compaction strategy
[ https://issues.apache.org/jira/browse/CASSANDRA-4049?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13396150#comment-13396150 ] Jonathan Ellis commented on CASSANDRA-4049: --- I'm not sure how you can give them meaningful names while keeping it pluggable and not tied to DSE, but I'm open to suggestions. Add generic way of adding SSTable components required custom compaction strategy Key: CASSANDRA-4049 URL: https://issues.apache.org/jira/browse/CASSANDRA-4049 Project: Cassandra Issue Type: New Feature Components: Core Reporter: Piotr Kołaczkowski Assignee: Piotr Kołaczkowski Priority: Minor Labels: compaction Fix For: 1.1.2 Attachments: compaction_strategy_cleanup.patch, component_patch.diff CFS compaction strategy coming up in the next DSE release needs to store some important information in Tombstones.db and RemovedKeys.db files, one per sstable. However, currently Cassandra issues warnings when these files are found in the data directory. Additionally, when switched to SizeTieredCompactionStrategy, the files are left in the data directory after compaction. The attached patch adds new components to the Component class so Cassandra knows about those files. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CASSANDRA-4049) Add generic way of adding SSTable components required custom compaction strategy
[ https://issues.apache.org/jira/browse/CASSANDRA-4049?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13244584#comment-13244584 ] Jonathan Ellis commented on CASSANDRA-4049: --- I'm a little nervous about this. Having descriptors know about only some components feels fragile and likely to cause bugs at some point. What if we added Custom1..Custom5 Type entries so that we're not just ignoring them? Add generic way of adding SSTable components required custom compaction strategy Key: CASSANDRA-4049 URL: https://issues.apache.org/jira/browse/CASSANDRA-4049 Project: Cassandra Issue Type: New Feature Components: Core Reporter: Piotr Kołaczkowski Priority: Minor Fix For: 1.0.10 Attachments: compaction_strategy_cleanup.patch, component_patch.diff CFS compaction strategy coming up in the next DSE release needs to store some important information in Tombstones.db and RemovedKeys.db files, one per sstable. However, currently Cassandra issues warnings when these files are found in the data directory. Additionally, when switched to SizeTieredCompactionStrategy, the files are left in the data directory after compaction. The attached patch adds new components to the Component class so Cassandra knows about those files. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
