[jira] [Commented] (CASSANDRA-11580) remove DatabaseDescriptor dependency from SegmentedFile
[
https://issues.apache.org/jira/browse/CASSANDRA-11580?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15487727#comment-15487727
]
Alex Petrov commented on CASSANDRA-11580:
-
Unfortunately, I can not use {{DatabaseDescriptor.toolInitialization}} as
that'd require the config file to be present (which I don't since it's a
client-only tool). I'll just wait until you have the patch for moving SSTable
initialization code to SStable and will improve from there. Thank you!
> remove DatabaseDescriptor dependency from SegmentedFile
> ---
>
> Key: CASSANDRA-11580
> URL: https://issues.apache.org/jira/browse/CASSANDRA-11580
> Project: Cassandra
> Issue Type: Sub-task
>Reporter: Yuki Morishita
>Assignee: Yuki Morishita
> Fix For: 3.10
>
>
> Several configurable parameters are pulled from {{DatabaseDescriptor}} from
> {{SegmentedFile}} and its subclasses.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-11580) remove DatabaseDescriptor dependency from SegmentedFile
[
https://issues.apache.org/jira/browse/CASSANDRA-11580?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15487404#comment-15487404
]
Yuki Morishita commented on CASSANDRA-11580:
[~ifesdjeen] We may be able to set default {{DiskOptimizationStrategy}} (which
is 'ssd' from cassandra.yaml) in {{DatabaseDescriptor}}, but I think there are
other configs to be set to properly open SSTable. So right now we need to do
{{DatabaseDescriptor.toolInitialization}} anyway which reads config from
{{cassandra.yaml}} file and set up that.
Later I want to move all SSTable related config from DD and put them in SSTable
config or something so we can have more control on configuring opening SSTable
programatically.
> remove DatabaseDescriptor dependency from SegmentedFile
> ---
>
> Key: CASSANDRA-11580
> URL: https://issues.apache.org/jira/browse/CASSANDRA-11580
> Project: Cassandra
> Issue Type: Sub-task
>Reporter: Yuki Morishita
>Assignee: Yuki Morishita
> Fix For: 3.10
>
>
> Several configurable parameters are pulled from {{DatabaseDescriptor}} from
> {{SegmentedFile}} and its subclasses.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-11580) remove DatabaseDescriptor dependency from SegmentedFile
[
https://issues.apache.org/jira/browse/CASSANDRA-11580?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15486531#comment-15486531
]
Alex Petrov commented on CASSANDRA-11580:
-
[~yukim] this has introduced a new assertion in SSTable, which makes it a tiny
bit harder to instantiate SSTable for tools, as previously the optimisation
strategy was not required (or at least the assertion was not there). Do you
think it makes sense to add some sort of default no-op optimiser that would
work for tools? Assuming it can always be overridden in {{DatabaseDescriptor}}
> remove DatabaseDescriptor dependency from SegmentedFile
> ---
>
> Key: CASSANDRA-11580
> URL: https://issues.apache.org/jira/browse/CASSANDRA-11580
> Project: Cassandra
> Issue Type: Sub-task
>Reporter: Yuki Morishita
>Assignee: Yuki Morishita
> Fix For: 3.10
>
>
> Several configurable parameters are pulled from {{DatabaseDescriptor}} from
> {{SegmentedFile}} and its subclasses.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-11580) remove DatabaseDescriptor dependency from SegmentedFile
[
https://issues.apache.org/jira/browse/CASSANDRA-11580?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15350339#comment-15350339
]
Yuki Morishita commented on CASSANDRA-11580:
Change and tests looks good to me, thanks!
The patch needs some rebasing, will do before code freeze.
> remove DatabaseDescriptor dependency from SegmentedFile
> ---
>
> Key: CASSANDRA-11580
> URL: https://issues.apache.org/jira/browse/CASSANDRA-11580
> Project: Cassandra
> Issue Type: Sub-task
>Reporter: Yuki Morishita
>Assignee: Yuki Morishita
> Fix For: 3.x
>
>
> Several configurable parameters are pulled from {{DatabaseDescriptor}} from
> {{SegmentedFile}} and its subclasses.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-11580) remove DatabaseDescriptor dependency from SegmentedFile
[
https://issues.apache.org/jira/browse/CASSANDRA-11580?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15335873#comment-15335873
]
Stefania commented on CASSANDRA-11580:
--
I've pushed some nits
[here|https://github.com/stef1927/cassandra/commit/ddee27868592076c7c30c285eb92938450eafe9e]:
mostly edits in the comments, unused imports and more restrictive access
modifiers in RAR. I've also fixed some resource management problems in unit
tests, and in the two new {{open()}} methods in case of exceptions. I've
rebased, which resulted in a couple of conflicts, especially in
{{CommitLogReader}} where some code has been moved around.
If you're +1 on my changes and the CI results are OK, then I'm also + 1 and we
can commit this:
|[patch|https://github.com/stef1927/cassandra/commits/11580]|[testall|http://cassci.datastax.com/view/Dev/view/stef1927/job/stef1927-11580-testall/]|[dtest|http://cassci.datastax.com/view/Dev/view/stef1927/job/stef1927-11580-dtest/]|
In terms of a better name for {{FileHandle}}, I'm also short of suggestions, it
is a factory for RAR basically, but one that owns resources, so the term
{{Handle}} is probably as good as {{Factory}} and given that it is created by a
builder, I tend to think that {{FileHandle}} is probably better then something
with the term factory in it. Feel free to start a discussion on IIRC re. a
better name if you want, once you are back from holiday, or commit with
{{FileHandle}}.
> remove DatabaseDescriptor dependency from SegmentedFile
> ---
>
> Key: CASSANDRA-11580
> URL: https://issues.apache.org/jira/browse/CASSANDRA-11580
> Project: Cassandra
> Issue Type: Sub-task
>Reporter: Yuki Morishita
>Assignee: Yuki Morishita
> Fix For: 3.x
>
>
> Several configurable parameters are pulled from {{DatabaseDescriptor}} from
> {{SegmentedFile}} and its subclasses.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-11580) remove DatabaseDescriptor dependency from SegmentedFile
[
https://issues.apache.org/jira/browse/CASSANDRA-11580?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15317744#comment-15317744
]
Yuki Morishita commented on CASSANDRA-11580:
I ended up renaming {{SegmentedFile}} to {{FileHandle}}. If anyone has better
name, I'm open to suggestion.
I think I fixed he points in the review, except: The EMPTY BufferHolder in
Rebuffered.java at line 62 can still be static final. variable in interface is
implicitly declared as public static final, and my IntelliJ gives me warning if
I left those.
||branch||testall||dtest||
|[11580|https://github.com/yukim/cassandra/tree/11580]|[testall|http://cassci.datastax.com/view/Dev/view/yukim/job/yukim-11580-testall/lastCompletedBuild/testReport/]|[dtest|http://cassci.datastax.com/view/Dev/view/yukim/job/yukim-11580-dtest/lastCompletedBuild/testReport/]|
> remove DatabaseDescriptor dependency from SegmentedFile
> ---
>
> Key: CASSANDRA-11580
> URL: https://issues.apache.org/jira/browse/CASSANDRA-11580
> Project: Cassandra
> Issue Type: Sub-task
>Reporter: Yuki Morishita
>Assignee: Yuki Morishita
> Fix For: 3.x
>
>
> Several configurable parameters are pulled from {{DatabaseDescriptor}} from
> {{SegmentedFile}} and its subclasses.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-11580) remove DatabaseDescriptor dependency from SegmentedFile
[
https://issues.apache.org/jira/browse/CASSANDRA-11580?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15299222#comment-15299222
]
Stefania commented on CASSANDRA-11580:
--
bq. I feel like the name {{SegmentedFile}} is not appropriate anymore, and it
just can be integrated with {{RandomAccessReader.Builder}}.
I totally agree, sounds like a good plan.
> remove DatabaseDescriptor dependency from SegmentedFile
> ---
>
> Key: CASSANDRA-11580
> URL: https://issues.apache.org/jira/browse/CASSANDRA-11580
> Project: Cassandra
> Issue Type: Sub-task
>Reporter: Yuki Morishita
>Assignee: Yuki Morishita
> Fix For: 3.x
>
>
> Several configurable parameters are pulled from {{DatabaseDescriptor}} from
> {{SegmentedFile}} and its subclasses.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-11580) remove DatabaseDescriptor dependency from SegmentedFile
[
https://issues.apache.org/jira/browse/CASSANDRA-11580?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15298958#comment-15298958
]
Yuki Morishita commented on CASSANDRA-11580:
Thanks for review.
I feel like the name {{SegmentedFile}} is not appropriate anymore, and it just
can be integrated with {{RandomAccessReader.Builder}}.
Let me work on that along with fixing your review points.
> remove DatabaseDescriptor dependency from SegmentedFile
> ---
>
> Key: CASSANDRA-11580
> URL: https://issues.apache.org/jira/browse/CASSANDRA-11580
> Project: Cassandra
> Issue Type: Sub-task
>Reporter: Yuki Morishita
>Assignee: Yuki Morishita
> Fix For: 3.x
>
>
> Several configurable parameters are pulled from {{DatabaseDescriptor}} from
> {{SegmentedFile}} and its subclasses.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-11580) remove DatabaseDescriptor dependency from SegmentedFile
[
https://issues.apache.org/jira/browse/CASSANDRA-11580?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15292830#comment-15292830
]
Stefania commented on CASSANDRA-11580:
--
Really great stuff Yuki, I could only find minor things and I had to look
really hard to find them :) The items marked as optional are left to your
decision since they are more down to individual stylistic preferences:
*SegmentedFile:*
* Class comments are no longer up-to-date
* protected accessors can be private
* Optional: class can be final
* Comments of onDiskLength mention SegmentIterator that no longer exists
* Trivial: alignment error in the constructor parameters at line 66
* Cleanup.Tidy(): the chunk cache is invalidated only when the metadata is not
null, this was the existing behavior but is it correct? Index files use a chunk
cache even if they are not compressed?
* Cleanup.Tidy(): can either metdata.close or the cache invalidation throw?
* Optional: should we rename metadata to compressionMetadata in Cleanup?
* Class comments of {{Builder}} are also no longer up-to-date
* Suppress resource warnings for rebufferer in Builder.complete() since it is
owned by the SegmentedFile
*Other files:*
* SStableReader ln 436: {{// special implementation of load to use non-pooled
SegmentedFile builders}} can be removed
* The EMPTY BufferHolder in Rebuffered.java at line 62 can still be static final
* SegmentedFileTest should be renamed to DiskOptimizationStrategyTest
* Optional: There is a bit of code duplications in BigTableWriter openFinal and
openForBatch as well as SSTableReader openForBatch and load. I wonder if we
could introduce helper methods in SSTable to create the index and data builders
and to create the index and data segmented files.
> remove DatabaseDescriptor dependency from SegmentedFile
> ---
>
> Key: CASSANDRA-11580
> URL: https://issues.apache.org/jira/browse/CASSANDRA-11580
> Project: Cassandra
> Issue Type: Sub-task
>Reporter: Yuki Morishita
>Assignee: Yuki Morishita
> Fix For: 3.x
>
>
> Several configurable parameters are pulled from {{DatabaseDescriptor}} from
> {{SegmentedFile}} and its subclasses.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-11580) remove DatabaseDescriptor dependency from SegmentedFile
[
https://issues.apache.org/jira/browse/CASSANDRA-11580?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15287601#comment-15287601
]
Yuki Morishita commented on CASSANDRA-11580:
||branch||testall||dtest||
|[11580|https://github.com/yukim/cassandra/tree/11580]|[testall|http://cassci.datastax.com/view/Dev/view/yukim/job/yukim-11580-testall/lastCompletedBuild/testReport/]|[dtest|http://cassci.datastax.com/view/Dev/view/yukim/job/yukim-11580-dtest/lastCompletedBuild/testReport/]|
The patch that removes DatabaseDescriptor dependency and {{ChunkCache}}
singleton access from {{SegmentedFile}}.
- All subclasses of {{SegmentedFile}} are merged into {{SegmentedFile}} since
difference among those is in {{Rebufferer}} after CASSANDRA-5863.
- Introduced {{DiskOptimizationStrategy}} for {{disk_optimization_strategy}}
config.
- {{SegmentedFile.Builder}}'s {{serializeBound}}/{{deserializedBound}} seem no
longer used, so I removed them.
> remove DatabaseDescriptor dependency from SegmentedFile
> ---
>
> Key: CASSANDRA-11580
> URL: https://issues.apache.org/jira/browse/CASSANDRA-11580
> Project: Cassandra
> Issue Type: Sub-task
>Reporter: Yuki Morishita
> Fix For: 3.x
>
>
> Several configurable parameters are pulled from {{DatabaseDescriptor}} from
> {{SegmentedFile}} and its subclasses.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
