Status Update
Comments
lu...@gmail.com <lu...@gmail.com> #2
This is not a replication
plugin problem, but a PluginLoader
issue:
- The replication plugin exposes an
ApiModule
in master - The
ApiModule
is loaded into theapiInjector
of theServerPlugin
- All the plugins with an
ApiModule
andapiInjector
are chained together
Whilst the start()
was fully implemented, the stop()
was not, see below:
protected void stop(PluginGuiceEnvironment env) {
if (serverManager != null) {
RequestContext oldContext = env.enter(this);
try {
serverManager.stop();
} finally {
env.exit(oldContext);
}
serverManager = null;
sysInjector = null;
sshInjector = null;
httpInjector = null;
}
}
There isn't anything that manages the apiInjector
therefore the unloading keeps the injections as-is. When the plugin is reloaded, then boom the injections are duplicated.
I know how to fix it, leave it with me!
lu...@gmail.com <lu...@gmail.com> #3
@Saša see an initial fix here:
This formally works, BUT isn't ready yet because in addition to keeping the old interface definition (acceptable IMHO) it also references the old classes, making the new reloaded code unreachable.
It is late now, will resume the efforts tomorrow.
lu...@gmail.com <lu...@gmail.com> #4
I believe the overall issue here is that, after ApiModule
, which is the lightweight version of the libModule exposing just an API.
The ApiModule
are supposed to be loaded only once and used as base injector for all plugins. When replication became an ApiModule
has lost its ability to be dynamically reloaded.
The best solution would be to introduce a new target in the replication plugin to generate the ApiModule
as standalone jar (e.g. replication-api.jar
) and leave the replication.jar
as a regular plugin.
We can also amend the x-plugin support in future versions of Gerrit to allow reloadable ApiModule
, but that is a much more complex exercise, because it involves the construction of a full dependency graph of the dependencies and reload all the dependent plugins when an ApiModule
is reoaded.
lu...@gmail.com <lu...@gmail.com> #5
Switching back the component to replication
; I have a possible solution that allows:
- Keeping the ability to use x-plugin communication to extend the replication plugin with
ApiModule
- Allow reloading the
replication.jar
dynamically and provide new code - Keeping the initial replication API implementation across reloads
@Saša @Darek please have a look at
lu...@gmail.com <lu...@gmail.com> #6
I have also finalised the check on the x-plugin management of the PluginLoader
for preventing a plugin with ApiModule
to be stopped, unloaded, reloaded or restarted:
They should be both ready for review!
na...@gmail.com <na...@gmail.com> #7
ap...@google.com <ap...@google.com> #8
Branch: master
commit 5e33f8344e3855478914a5c530a327f888bd8c8e
Author: Luca Milanesio <luca.milanesio@gmail.com>
Date: Sat Feb 10 21:55:12 2024
Decouple replication-api.jar from the main replication plugin
The Change Ib7a04eea5 made the whole replication plugin an ApiModule
which becomes the base injector for all plugins and forbids reloads,
(see the Iac2851022ea for enforcing it at Gerrit level)
even when there is no intention to use th extension points.
All plugins that expose an ApiModule have the following limitations:
- Cannot add extra dependencies
- Cannot be reloaded
- Are the base injectors for all plugins
The above limitations aren't acceptable for the whole replication
plugin when used standalone without extensions.
Define a new replication-api target that generates the sole ApiModule
needed for extending the replication plugin functionality.
All the classes that are part of the replication-api are moved
into a separate c.g.g.p.replication.api package so that can be
included in the replication-api.jar plugin.
The gerrit.war will continue to include only the replication.jar which
will still work out of the box. Anyone willing to customise its behaviour
through the API will have to build the replication-api target.
Also fix the MergedConfigResource that was assuming that the ApiModule
was always installed and was binding DynamicItem<ReplicationConfigOverrides>.
Bug:
Release-Notes: Decouple replication-api.jar from the main replication plugin
Change-Id: Ic432fc77daf1162368abd65f64c463e4ef4f5d6d
M BUILD
M src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecorator.java
M src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadRunnable.java
M src/main/java/com/googlesource/gerrit/plugins/replication/AutoReloadSecureCredentialsFactoryDecorator.java
M src/main/java/com/googlesource/gerrit/plugins/replication/CreateProjectTask.java
M src/main/java/com/googlesource/gerrit/plugins/replication/DestinationsCollection.java
M src/main/java/com/googlesource/gerrit/plugins/replication/FileConfigResource.java
M src/main/java/com/googlesource/gerrit/plugins/replication/ListCommand.java
M src/main/java/com/googlesource/gerrit/plugins/replication/MergedConfigResource.java
M src/main/java/com/googlesource/gerrit/plugins/replication/OnStartStop.java
M src/main/java/com/googlesource/gerrit/plugins/replication/PushOne.java
M src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationConfigImpl.java
M src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationConfigModule.java
M src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationDestinations.java
M src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationExtensionPointModule.java
M src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationQueue.java
M src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationRemotesUpdater.java
M src/main/java/com/googlesource/gerrit/plugins/replication/ReplicationTasksStorage.java
M src/main/java/com/googlesource/gerrit/plugins/replication/SshHelper.java
M src/main/java/com/googlesource/gerrit/plugins/replication/api/ApiModule.java
M src/main/java/com/googlesource/gerrit/plugins/replication/api/ConfigResource.java
M src/main/java/com/googlesource/gerrit/plugins/replication/api/ReplicationConfig.java
M src/main/java/com/googlesource/gerrit/plugins/replication/api/ReplicationConfigOverrides.java
M src/main/java/com/googlesource/gerrit/plugins/replication/api/ReplicationPushFilter.java
M src/main/resources/Documentation/extension-point.md
M src/test/java/com/googlesource/gerrit/plugins/replication/AbstractConfigTest.java
M src/test/java/com/googlesource/gerrit/plugins/replication/AutoReloadConfigDecoratorTest.java
M src/test/java/com/googlesource/gerrit/plugins/replication/FanoutConfigResourceTest.java
M src/test/java/com/googlesource/gerrit/plugins/replication/FileConfigResourceTest.java
M src/test/java/com/googlesource/gerrit/plugins/replication/MergedConfigResourceTest.java
M src/test/java/com/googlesource/gerrit/plugins/replication/PushOneTest.java
M src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationConfigImplTest.java
M src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationRemotesUpdaterTest.java
M src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationStorageDaemon.java
M src/test/java/com/googlesource/gerrit/plugins/replication/ReplicationStorageIT.java
M src/test/java/com/googlesource/gerrit/plugins/replication/TestReplicationModule.java
ap...@google.com <ap...@google.com> #9
Branch: stable-3.9
commit 7d4ace2c7718c9632ad1f2d9dcbbb0a035642bbb
Author: Luca Milanesio <luca.milanesio@gmail.com>
Date: Sat Feb 10 01:21:08 2024
Do not allow unload/restart plugins with ApiModule
The ApiModule from plugins is used for creating the
base injectors for all other plugins and cannot be
unloaded without potentially breaking all the other
plugins that depend on them.
Block the start of the new plugin for avoiding
leaving the overall Guice stack in an inconsistent
state. Plugins with ApiModule need to be effectively
considered mandatory.
Release-Notes: Prevent reload of plugins with ApiModule
Bug:
Change-Id: Iac2851022ea34e52014c519eeef70ce6028f4fda
M java/com/google/gerrit/server/plugins/DisablePlugin.java
M java/com/google/gerrit/server/plugins/PluginLoader.java
M java/com/google/gerrit/server/plugins/ServerPlugin.java
M java/com/google/gerrit/sshd/commands/PluginRemoveCommand.java
Description
It is not possible to reload the replication plugin. How to reproduce: