Start to Refactor CassandraStorage #1294

Miles-Garnsey · 2023-05-05T03:23:20Z

This PR begins the refactoring process on CassandraStorage as discussed in #1265. The long term goals are:

Split up the monolithic IStorage, IDistributedStorage interfaces and monolithic CassandraStorage and MemoryStorage classes so that we end up with smaller DAO classes mapping more closely to the types defined in core.
Ensure return types are aligned with types from core where possible and are never ResultSets (which are almost untyped and hard to work with).
Bring the codebase into a more manageable state where it is clear which types are collaborating with other types.

This PR is a very small step in that direction, and simply delegates many of the storage operations from CassandraStorage to DAO types.

Further steps need to be taken to reflect the broad cuts made here in the interfaces and in MemoryStorage, before creating smaller interfaces that the new DAO types adhere to and passing those interfaces (instead of e.g. the larger monolithic IStorage) to consumers.

…kstyle.

github-actions · 2023-05-05T03:23:34Z

No linked issues found. Please add the corresponding issues in the pull request description.
Use GitHub automation to close the issue when a PR is merged

…usterDao, removing ClusterDao's dependancy on RepairRunDao.

…leDao for schedule prepared statements.

…tibility.

adejanovski

Couple of requested changes to unblock testing.

src/server/src/main/java/io/cassandrareaper/storage/cassandra/CassandraStorage.java

src/server/src/main/java/io/cassandrareaper/storage/cassandra/RepairScheduleDao.java

…CassandraStorage.java Ensure migrations completed before statement preparation. Co-authored-by: Alexander Dejanovski <alex.dejanovski@datastax.com>

…RepairScheduleDao.java Ensure ReairSchedule prepared statements are actually prepared. Co-authored-by: Alexander Dejanovski <alex.dejanovski@datastax.com>

Miles-Garnsey · 2023-05-10T04:50:00Z

@adejanovski thanks for the input, the tests now pass. Can you please complete your review, and especially take a look at what I've done with the acceptance tests? I've removed all references to Cassandra 2.x to get things running, but maybe we can leave the cases where the cluster to be repaired is Cassandra 2.x, as long as the backend is 3.x+?

I do note that the tests against trunk all seem to be failing, I'm not sure if that is a problem or not given that trunk is presumably often unstable (or API incompatible with Reaper perhaps? Not sure...)

adejanovski

Very nice refactorings!
There are some of the tests in the memory jobs that need to be re-introduced and a few comments that should be deleted.
Otherwise LGTM.
You can merge the PR once the above points have been addressed and CI passes.

.github/workflows/ci.yaml

src/server/src/main/java/io/cassandrareaper/storage/cassandra/CassandraStorage.java

Miles-Garnsey · 2023-05-11T01:08:51Z

All changes made as requested, with removal of dateOf references to ensure compatibility with trunk. I'll merge once/if tests pass.

Miles-Garnsey added 13 commits May 2, 2023 18:59

Refactor storage.cassandra package.

d4eecf4

Remove metals via gitignore.

053db96

Shuffle CassandraStorage into storage.cassandra. Fix imports for chec…

676f836

…kstyle.

Split out RepairRunDAO into a delegate class.

33127fb

Split out RepairSegmentDAO into a delegate class.

e025bd1

Checkstyle.

b0b4ff9

Further separate RepairRunDao, RepairSegmentDao from CassandraStorage.

0281efe

RepairUnitDao.

2276b07

Move remaining statement preparation functions out into DAOs.

793256c

Checkstyle.

80540f4

RepairScheduleDAO.

c444b04

RepairUnitDao no longer depends on CassandraStorage at all.

d3fd6dd

Checkstyle.

3f5e1de

Miles-Garnsey added 16 commits May 5, 2023 13:55

Split out EventsDao and ClusterDao.

9da4f81

Move deleteRepairRunByClusterPrepStmt out of RepairRunDao and into Cl…

fea9a71

…usterDao, removing ClusterDao's dependancy on RepairRunDao.

MetricsDao.

50b0800

Eliminate RepairRunDao dependancy on CassandraStorage.

7d42c19

Factor prepareMetricsStatements out of CassandraStorage.

b9f7d07

Checkstyle, rearrange some arguments.

91615a1

MigrationManager factored out.

6933cae

prepareStatements moved out of CassandraStorage and into RepairSchedu…

a7cbf43

…leDao for schedule prepared statements.

Concurrency control methods factored out into Concurrency.

00bc491

Make Migration class constructor private.

efb9cd4

RepairSegmentDao no longer depends on CassandraStorage.

3d75362

SnapshotDao.

0a21a56

OperationsDao.

bf43655

Remove version info from RepairSegmentDao and break Cassandra 2 compa…

c38a6fd

…tibility.

Remove Cassandra 2.x backend from tests.

e27374c

Checkstyle...

89650ae

Remove more Cassandra 2.x things.

9f191a1

adejanovski requested changes May 9, 2023

View reviewed changes

src/server/src/main/java/io/cassandrareaper/storage/cassandra/CassandraStorage.java Outdated Show resolved Hide resolved

src/server/src/main/java/io/cassandrareaper/storage/cassandra/RepairScheduleDao.java Show resolved Hide resolved

Miles-Garnsey and others added 2 commits May 10, 2023 10:25

Update src/server/src/main/java/io/cassandrareaper/storage/cassandra/…

517b5b0

…CassandraStorage.java Ensure migrations completed before statement preparation. Co-authored-by: Alexander Dejanovski <alex.dejanovski@datastax.com>

Update src/server/src/main/java/io/cassandrareaper/storage/cassandra/…

f9dd001

…RepairScheduleDao.java Ensure ReairSchedule prepared statements are actually prepared. Co-authored-by: Alexander Dejanovski <alex.dejanovski@datastax.com>

Miles-Garnsey force-pushed the refactor/daos branch from 2298d18 to f9dd001 Compare May 10, 2023 03:18

adejanovski previously approved these changes May 10, 2023

View reviewed changes

Miles-Garnsey added 3 commits May 11, 2023 10:30

Remove all usages of dateOf function (removed in Cassandra trunk).

37549a1

Reinstate memory backends for tests on 1.x, 2.x.

a4562f7

Remove comments that were copied during refactor.

c4f94b7

Miles-Garnsey dismissed adejanovski’s stale review via c4f94b7 May 11, 2023 01:07

Miles-Garnsey marked this pull request as ready for review May 11, 2023 01:08

Miles-Garnsey and others added 3 commits May 16, 2023 18:05

Remove tests against trunk to prevent them blocking downstream tests.

f560bb1

Fully disable trunk tests and remove elassandra ones

6959082

reintroduce missing includes for the memory tests

61b3f4b

adejanovski approved these changes May 16, 2023

View reviewed changes

adejanovski merged commit aae9ec5 into thelastpickle:master May 16, 2023
22 checks passed

adejanovski pushed a commit that referenced this pull request Jun 22, 2023

Refactor CassandraStorage (#1294)

bf4a36c

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Start to Refactor CassandraStorage #1294

Start to Refactor CassandraStorage #1294

Miles-Garnsey commented May 5, 2023

github-actions bot commented May 5, 2023

adejanovski left a comment

Miles-Garnsey commented May 10, 2023

adejanovski left a comment

Miles-Garnsey commented May 11, 2023

Start to Refactor CassandraStorage #1294

Start to Refactor CassandraStorage #1294

Conversation

Miles-Garnsey commented May 5, 2023

github-actions bot commented May 5, 2023

adejanovski left a comment

Choose a reason for hiding this comment

Miles-Garnsey commented May 10, 2023

adejanovski left a comment

Choose a reason for hiding this comment

Miles-Garnsey commented May 11, 2023