[
https://issues.apache.org/jira/browse/CASSANDRA-15196?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Josh McKenzie updated CASSANDRA-15196:
--------------------------------------
Description:
{{April 2023 update}}
With the push towards stabilizing ASF CI, one of the things we can do is remove
the total runtime of our tests. CDC's segment allocation has been in the
ecosystem for a long while now and has proven stable in the "CDC enabled on the
node but not the table" case, so unifying the hierarchy to a single allocator
and removing the test-cdc target in ant should both help us clean up some debt
in the codebase and also streamline our CI processes. Along with that, we can
take this chance to tidy up some of the internals of the implementation as
previously mentioned on this ticket.
{{ORIGINAL DESCRIPTION}}
Back when I wrote CASSANDRA-8844 and CASSANDRA-12148, a few things about the
CommitLog stood out to me that I wasn't in love with. At the time I was much
more of the mind of "When in Rome..." regarding some of our structures, our
naming, and specifically regarding our penchant for inheritance vs.
composition. Turns out I have an itch here I need to scratch.
This patch refactors the CommitLog in a few key ways:
* Removes inheritence for CommitLogSegmentManagerX, instead has a single
CommitLogSegmentManager and a CommitLogSegmentAllocator interface
** This interface is implemented by CommitLogSegmentAllocatorStandard and
CommitLogSegmentAllocatorCDC
* Renames a few variables and methods within the segment manager to make their
purpose and role more clear from names alone (hopefully):
** allocatingFrom --> activeSegment
** allocatingFrom() --> getActiveSegment()
** advanceAllocatingFrom() – switchToNewSegment()
** activeSegments --> unflushedSegments
** getActiveSegments --> getUnflushedSegments()
** awaitAvailableSegment(...) --> awaitSegmentAllocation(...)
** isStillAllocating() --> hasRoom()
* Reorders some of the "allocation-and-compare-in-while-loop-decl) to more
idiomatic style
* Adds some comments to various under-documented methods, mostly related to
CDC and timing
-As far as we are in the 4.0 testing, I don't expect this to hit pre 4.0. Given
it's a relatively minor refactor in well-tested code, doesn't change
functionality, and doesn't impact any of the publicly exposed APIs in the
CommitLog ecosystem, I'd be comfortable with it going in a minor release after
4.0 is out. But I'm open to alternative viewpoints.-
-Have run unit tests locally w/out issue on both regular and cdc-targets; need
to get CI against the branch internally for full dtest run and I'll post when
it's done. Just wanted to get visibility to this out there in case anyone was
curious or thinking of working on the same thing.-
[ Link to branch on
github|https://github.com/apache/cassandra/compare/trunk...josh-mckenzie:clean_cl_comp]
was:
Back when I wrote CASSANDRA-8844 and CASSANDRA-12148, a few things about the
CommitLog stood out to me that I wasn't in love with. At the time I was much
more of the mind of "When in Rome..." regarding some of our structures, our
naming, and specifically regarding our penchant for inheritance vs.
composition. Turns out I have an itch here I need to scratch.
This patch refactors the CommitLog in a few key ways:
* Removes inheritence for CommitLogSegmentManagerX, instead has a single
CommitLogSegmentManager and a CommitLogSegmentAllocator interface
** This interface is implemented by CommitLogSegmentAllocatorStandard and
CommitLogSegmentAllocatorCDC
* Renames a few variables and methods within the segment manager to make their
purpose and role more clear from names alone (hopefully):
** allocatingFrom --> activeSegment
** allocatingFrom() --> getActiveSegment()
** advanceAllocatingFrom() – switchToNewSegment()
** activeSegments --> unflushedSegments
** getActiveSegments --> getUnflushedSegments()
** awaitAvailableSegment(...) --> awaitSegmentAllocation(...)
** isStillAllocating() --> hasRoom()
* Reorders some of the "allocation-and-compare-in-while-loop-decl) to more
idiomatic style
* Adds some comments to various under-documented methods, mostly related to
CDC and timing
As far as we are in the 4.0 testing, I don't expect this to hit pre 4.0. Given
it's a relatively minor refactor in well-tested code, doesn't change
functionality, and doesn't impact any of the publicly exposed APIs in the
CommitLog ecosystem, I'd be comfortable with it going in a minor release after
4.0 is out. But I'm open to alternative viewpoints.
Have run unit tests locally w/out issue on both regular and cdc-targets; need
to get CI against the branch internally for full dtest run and I'll post when
it's done. Just wanted to get visibility to this out there in case anyone was
curious or thinking of working on the same thing.
[ Link to branch on
github|https://github.com/apache/cassandra/compare/trunk...josh-mckenzie:clean_cl_comp]
> Unify CommitLogSegment allocaiton to one hierarchy and remove test-cdc
> testing target
> -------------------------------------------------------------------------------------
>
> Key: CASSANDRA-15196
> URL: https://issues.apache.org/jira/browse/CASSANDRA-15196
> Project: Cassandra
> Issue Type: Improvement
> Components: Local/Commit Log
> Reporter: Joshua McKenzie
> Assignee: Josh McKenzie
> Priority: Normal
> Fix For: 5.x
>
>
> {{April 2023 update}}
> With the push towards stabilizing ASF CI, one of the things we can do is
> remove the total runtime of our tests. CDC's segment allocation has been in
> the ecosystem for a long while now and has proven stable in the "CDC enabled
> on the node but not the table" case, so unifying the hierarchy to a single
> allocator and removing the test-cdc target in ant should both help us clean
> up some debt in the codebase and also streamline our CI processes. Along with
> that, we can take this chance to tidy up some of the internals of the
> implementation as previously mentioned on this ticket.
>
> {{ORIGINAL DESCRIPTION}}
> Back when I wrote CASSANDRA-8844 and CASSANDRA-12148, a few things about the
> CommitLog stood out to me that I wasn't in love with. At the time I was much
> more of the mind of "When in Rome..." regarding some of our structures, our
> naming, and specifically regarding our penchant for inheritance vs.
> composition. Turns out I have an itch here I need to scratch.
> This patch refactors the CommitLog in a few key ways:
> * Removes inheritence for CommitLogSegmentManagerX, instead has a single
> CommitLogSegmentManager and a CommitLogSegmentAllocator interface
> ** This interface is implemented by CommitLogSegmentAllocatorStandard and
> CommitLogSegmentAllocatorCDC
> * Renames a few variables and methods within the segment manager to make
> their purpose and role more clear from names alone (hopefully):
> ** allocatingFrom --> activeSegment
> ** allocatingFrom() --> getActiveSegment()
> ** advanceAllocatingFrom() – switchToNewSegment()
> ** activeSegments --> unflushedSegments
> ** getActiveSegments --> getUnflushedSegments()
> ** awaitAvailableSegment(...) --> awaitSegmentAllocation(...)
> ** isStillAllocating() --> hasRoom()
> * Reorders some of the "allocation-and-compare-in-while-loop-decl) to more
> idiomatic style
> * Adds some comments to various under-documented methods, mostly related to
> CDC and timing
> -As far as we are in the 4.0 testing, I don't expect this to hit pre 4.0.
> Given it's a relatively minor refactor in well-tested code, doesn't change
> functionality, and doesn't impact any of the publicly exposed APIs in the
> CommitLog ecosystem, I'd be comfortable with it going in a minor release
> after 4.0 is out. But I'm open to alternative viewpoints.-
> -Have run unit tests locally w/out issue on both regular and cdc-targets;
> need to get CI against the branch internally for full dtest run and I'll post
> when it's done. Just wanted to get visibility to this out there in case
> anyone was curious or thinking of working on the same thing.-
> [ Link to branch on
> github|https://github.com/apache/cassandra/compare/trunk...josh-mckenzie:clean_cl_comp]
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
