Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework view services and system-distributed-keyspace dependencies #18706

Conversation

xemul
Copy link
Contributor

@xemul xemul commented May 16, 2024

The system-distributed-keyspace and view-update-generator often go in pair, because streaming, repair and sstables-loader (via distributed-loader) need them booth to check if sstable is staging and register it if it's such. The check is performed by messing directly with system_distributed.view_build_status table, and the registration happens via view-update-generator.

That's not nice, other services shouldn't know that view status is kept in system table. Also view-update-generator is a service to generae and push view updates, the fact that it keeps staging sstables list is the implementation detail.

This PR replaces dependencies on the mentioned pair of services with the single dependency on view-builder (repair, sstables-loader and stream-manager are enlightened) and hides the view building-vs-staging details inside the view_builder.

Along the way, some simplification of repair_writer_impl class is done.

@xemul xemul added the backport/none Backport is not required label May 16, 2024
@scylladb-promoter
Copy link
Contributor

🟢 CI State: SUCCESS

✅ - Build
✅ - Container Test
✅ - dtest
✅ - dtest with topology changes
✅ - Unit Tests

Build Details:

  • Duration: 6 hr 39 min
  • Builder: i-023bfb81dc4a3d3d3 (r5ad.8xlarge)

Copy link
Contributor

@denesb denesb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Has conflicts, please rebase.

xemul added 15 commits May 23, 2024 13:32
When constructed, the class copies local references to services just to
push them into make_repair_writer() later in the same initializers list.
There's no need in keeping those references.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
This service is on its own, nothing depends on it. Neither it can work
before system distributed keyspace is started, so move it lower.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Preparation to next patches, they'll make use of this new argument

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Preparation patch, next patches will make use of this new member

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Continuation of the previous patch. Repair itself doesn't need it, but
streaming consumer does.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
This helper checks if there's an ongoing build of a view, and it's in
fact internal to view-builder, who keeps its status in one of its
system tables.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Callers of it had just checked if an sstable still has some views
building, so the should talk to view-builder to register the sstable
that's now considered to be staging.

Effectively. this is to hide the view-update-generator from other
services and make them communicate with the builder only.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
…e_generator

Now all the code is happy with view_builder and can be shortened

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
…enerator

Now all the code is happy with view_builder and can be shortened

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Now all the code is happy with view_builder and can be shortened

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Now all the code is happy with view_builder and can be shortened

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
…nerator

Now all the code is happy with view_builder and can be shortened

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
@xemul xemul force-pushed the br-view-builder-staging-sstables-dependency branch from c3625ef to 8906126 Compare May 23, 2024 10:46
@xemul
Copy link
Contributor Author

xemul commented May 23, 2024

upd:

@scylladb-promoter
Copy link
Contributor

🟢 CI State: SUCCESS

✅ - Build
✅ - Container Test
✅ - dtest with topology changes
✅ - dtest
✅ - Unit Tests

Build Details:

  • Duration: 4 hr 14 min
  • Builder: i-0bd665ece5a0aaeb0 (m5d.8xlarge)

@scylladb-promoter scylladb-promoter merged commit 47dbf23 into scylladb:master May 27, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/none Backport is not required promoted-to-master
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants