cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jonathan Ellis (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CASSANDRA-4049) Add generic way of adding SSTable components required custom compaction strategy
Date Fri, 31 Aug 2012 14:14:07 GMT

    [ https://issues.apache.org/jira/browse/CASSANDRA-4049?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=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)
        {
            Set<Component> 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

Mime
View raw message