Make RepositoriesService project-aware #129821

ywangd · 2025-06-23T03:14:45Z

This PR makes RepositoriesService project aware so that the basic Put, Get, Delete and Verify repository actions are now project scoped.

It intentionally leaves the following aspects out of scope for the current changes:

Repository stats reporting
Repository clean-up, analysis and integrity verification
Repository usages for searchable snapshots and CCR

They will be worked on separately. One main reason for leaving them out is that they are not needed by OBS which is currently blocked by repository/snapshot changes. They may also have their own complexities, e.g. stats reporting.

Resolves: ES-10478

ywangd · 2025-06-23T03:20:46Z

server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java

+    /**
+     * Apply changes for one project. The project can be either newly added, removed or an existing one.
+     *
+     * @param version The cluster state version of the change.
+     * @param state The current project state, or {@code null} if the project was removed.
+     * @param previousState the previous project state, or {@code null} if the project was newly added.
+     */
+    private void applyProjectState(long version, @Nullable ProjectState state, @Nullable ProjectState previousState) {
+        assert state != null || previousState != null : "state and previousState cannot both be null";
+        assert state == null || assertReadonlyRepositoriesNotInUseForWrites(state);


The changeset of this method can be a bit deceptively large due to method extraction and indentation difference. It is largely the same as the exisitng applyClusterState method with some additional logic for new and removed projects.

nit: Any chance we could break up this method so not both states can be nullable so it is a bit more readable? I find it rather odd but I understand that's because you want to keep this logic the same with just some null checks.

I initially started with breaking up the method for added, removed and existing projects. But found out that processing existing projects requires all logic (because repo can still be added/removed for existing projects) which led to duplication and felt non-ideal. That said, I tried it again by separating "removed projects" out in its own method and it seems to be a better middle-ground (compared to 1 or 3 methods). Please let me know whether it looks better to you. f62e3fe

ywangd · 2025-06-23T03:21:35Z

server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java

Most changes are cascading effects from adding projectId to this class and its methods.

…project-aware

ywangd · 2025-06-23T06:43:05Z

x-pack/qa/multi-project/core-rest-tests-with-multiple-projects/build.gradle

+    '^cat.repositories/*/*',
    '^cat.snapshots/*/*',
    '^cluster.desired_balance/10_basic/*',
+    '^cluster.stats/10_basic/snapshot stats reported in get cluster stats',


Re-mute these repository related YAML tests since they require either create, get or delete snapshots to work in MP setup. Previously these tests passed but it was in fact incorrect because they always target the default project.

ywangd · 2025-06-23T06:43:46Z

x-pack/qa/multi-project/core-rest-tests-with-multiple-projects/build.gradle

-    '^snapshot.get_repository/20_repository_uuid/*',
+    '^snapshot.get_repository/*/*',


Same mute here. They will get unmute again once repositoriy and snapshot are truly MP ready.

elasticsearchmachine · 2025-06-23T06:44:34Z

Pinging @elastic/es-distributed-coordination (Team:Distributed Coordination)

pxsalehi · 2025-06-23T11:58:44Z

Related to making snapshotting code project aware, I've opened ES-12121 recently. Not saying it needs to be done here, but since you've gone through the code for making things project aware, it might be worth taking a look into where the block checks could be also added, and add some note to that ticket if you thing it helps.

I see you've done some of that here. If you think that's it for ES-12121, feel free to close that one too. Thanks.

pxsalehi · 2025-06-23T12:03:16Z

.../elasticsearch/action/admin/cluster/repositories/reservedstate/ReservedRepositoryAction.java

@@ -60,7 +62,9 @@ public Collection<PutRepositoryRequest> prepare(Object input) {
        for (var repositoryRequest : repositories) {
            validate(repositoryRequest);
            RepositoriesService.validateRepositoryName(repositoryRequest.name());
-            repositoriesService.validateRepositoryCanBeCreated(repositoryRequest);
+            @FixForMultiProject


even if it is very obvious to you, I'd recommend adding a comment/description to these annotations so it is immediately clear what the action item is, or any other detail/suggestion you might have to resolve it. I've left the other annotations that have no comment/description uncommented but please do that for them too.

Sure see 75f2b75

pxsalehi · 2025-06-23T12:09:13Z

server/src/main/java/org/elasticsearch/cluster/ClusterChangedEvent.java

+    public Set<ProjectId> commonProjects() {
+        return projectsDelta.common(state);


does this fit with the existing pattern of deltas? could you just use a set operation where ever this is needed (currentProjects - added)? Seems a bit odd to have this here. Maybe as a static method on two given metadata?

Yeah I was not super happy with the changes either. I was trying to achieve (1) compute the common set once and (2) not storing the common set separately if it is the same as all projects. I reverted the changes and let the caller compute the common set of projects ca0ea7a

pxsalehi · 2025-06-23T12:16:49Z

...ain/java/org/elasticsearch/xpack/ccr/action/repositories/PutInternalCcrRepositoryAction.java

-            repositoriesService.registerInternalRepository(request.getName(), request.getType());
+            @FixForMultiProject
+            final var projectId = ProjectId.DEFAULT;
+            repositoriesService.registerInternalRepository(projectId, request.getName(), request.getType());


what's there to fix? i imagine this will only be default.

Maybe if CCR is never expected to work in multi-project. With the initial namespacing effort, we generally agreed that it is better if all elasticsearch is project-aware because it is easier to reason with instead of having to remember which part is and which part is not. For example, we made ILM project aware even though it is not used in serverless. There are certainly pratical limits on how much we can push this kinda of effort. But leaving an annotation here seems to be the right thing to do until we can firmly decide what to do with CCR. I also added an description and linked the issue for the annotation.

pxsalehi · 2025-06-23T12:18:26Z

server/src/main/java/org/elasticsearch/snapshots/RestoreService.java

@@ -602,7 +602,9 @@ static void refreshRepositoryUuids(
                return;
            }

-            for (Repository repository : repositoriesService.getRepositories().values()) {
+            @FixForMultiProject


so making restore project-aware is also deferred to a follow up?

Yes restore is tracked by ES-10228. It's also not needed by OBS.

pxsalehi · 2025-06-23T12:25:28Z

server/src/main/java/org/elasticsearch/repositories/RepositoriesService.java

+    /**
+     * Apply changes for one project. The project can be either newly added, removed or an existing one.
+     *
+     * @param version The cluster state version of the change.
+     * @param state The current project state, or {@code null} if the project was removed.
+     * @param previousState the previous project state, or {@code null} if the project was newly added.
+     */
+    private void applyProjectState(long version, @Nullable ProjectState state, @Nullable ProjectState previousState) {
+        assert state != null || previousState != null : "state and previousState cannot both be null";
+        assert state == null || assertReadonlyRepositoriesNotInUseForWrites(state);


nit: Any chance we could break up this method so not both states can be nullable so it is a bit more readable? I find it rather odd but I understand that's because you want to keep this logic the same with just some null checks.

ywangd · 2025-06-24T01:44:15Z

I didn't know the existence of ES-12121. Thanks for the reminder. I did intend to address the issue along the way while working on related APIs. It's not completed yet since we still need the same treatmeant for other snapshot and retore APIs. I added it as a linked item to ES-10166, ES-10227, ES-12158 and ES-10478. I will close ES-12121 once all the linked items are completed.

…project-aware

This PR makes RepositoriesService project aware so that the basic Put, Get, Delete and Verify repository actions are now project scoped. It intentionally leaves the following aspects out of scope for the current changes: * Repository stats reporting * Repository clean-up, analysis and integrity verification * Repository usages for searchable snapshots and CCR They will be worked on separately. One main reason for leaving them out is that they are not needed by OBS which is currently blocked by repository/snapshot changes. They may also have their own complexities, e.g. stats reporting. Resolves: ES-10478

wip

dc2451f

elasticsearchmachine added the v9.1.0 label Jun 23, 2025

ywangd commented Jun 23, 2025

View reviewed changes

ywangd added 3 commits June 23, 2025 13:26

more test

d2f4f3d

Merge remote-tracking branch 'origin/main' into repositories-service-…

8555bce

…project-aware

fix and mute tests

2e32d6c

ywangd commented Jun 23, 2025

View reviewed changes

ywangd added >non-issue :Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs labels Jun 23, 2025

ywangd marked this pull request as ready for review June 23, 2025 06:44

ywangd requested review from DaveCTurner and pxsalehi June 23, 2025 06:44

elasticsearchmachine added the Team:Distributed Coordination Meta label for Distributed Coordination team label Jun 23, 2025

elasticsearchmachine added the serverless-linked Added by automation, don't add manually label Jun 23, 2025

ywangd added 2 commits June 23, 2025 17:13

tweak

272266d

mute

c317b60

pxsalehi reviewed Jun 23, 2025

View reviewed changes

ywangd and others added 4 commits June 24, 2025 10:32

revert changes to ClusterChangedEvent

ca0ea7a

expand annotations

75f2b75

[CI] Auto commit changes from spotless

bf1b67d

refactor

f62e3fe

ywangd requested a review from pxsalehi June 24, 2025 01:44

ywangd added 2 commits June 24, 2025 12:20

Merge remote-tracking branch 'origin/main' into repositories-service-…

cbf06b7

…project-aware

Merge remote-tracking branch 'origin/main' into repositories-service-…

3b77702

…project-aware

pxsalehi approved these changes Jun 24, 2025

View reviewed changes

Merge remote-tracking branch 'origin/main' into repositories-service-…

4b2ab42

…project-aware

ywangd added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Jun 24, 2025

elasticsearchmachine merged commit e1c930f into elastic:main Jun 25, 2025
32 checks passed

ywangd deleted the repositories-service-project-aware branch June 25, 2025 00:34

		'^snapshot.get_repository/20_repository_uuid/*',
		'^snapshot.get_repository//',

		public Set<ProjectId> commonProjects() {
		return projectsDelta.common(state);

Make RepositoriesService project-aware #129821

Make RepositoriesService project-aware #129821

Uh oh!

Conversation

ywangd commented Jun 23, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elasticsearchmachine commented Jun 23, 2025

Uh oh!

pxsalehi commented Jun 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ywangd commented Jun 24, 2025

Uh oh!

Uh oh!

Uh oh!

pxsalehi commented Jun 23, 2025 •

edited

Loading