Skip to content

Commit d055df2

Browse files
s4ch1ns4ch1n
authored
[Enhancement] Make plugin install/uninstall idempotent (#75563)
1 parent 8b9fc20 commit d055df2

10 files changed

Lines changed: 207 additions & 7 deletions

File tree

‎docs/en/sql-reference/sql-statements/cluster-management/plugin/INSTALL_PLUGIN.md‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,11 @@ This operation requires the SYSTEM-level PLUGIN privilege. You can follow the in
1616
## Syntax
1717

1818
```sql
19-
INSTALL PLUGIN FROM [source] [PROPERTIES ("key"="value", ...)]
19+
INSTALL PLUGIN [IF NOT EXISTS] FROM [source] [PROPERTIES ("key"="value", ...)]
2020
```
2121

22+
**IF NOT EXISTS**: If specified, the statement succeeds silently when the plugin is already installed instead of returning an error.
23+
2224
3 types of sources are supported:
2325

2426
```plain text
@@ -54,3 +56,9 @@ PROPERTIES supports setting some configurations of plugins, such as setting the
5456
```sql
5557
INSTALL PLUGIN FROM "http://mywebsite.com/plugin.zip" PROPERTIES("md5sum" = "73877f6029216f4314d712086a146570");
5658
```
59+
60+
5. Install a plugin if it is not already installed:
61+
62+
```sql
63+
INSTALL PLUGIN IF NOT EXISTS FROM "/home/users/starrocks/auditdemo.zip";
64+
```

‎docs/en/sql-reference/sql-statements/cluster-management/plugin/UNINSTALL_PLUGIN.md‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,11 @@ This operation requires the SYSTEM-level PLUGIN privilege. You can follow the in
1616
## Syntax
1717

1818
```SQL
19-
UNINSTALL PLUGIN <plugin_name>
19+
UNINSTALL PLUGIN [IF EXISTS] <plugin_name>
2020
```
2121

22+
**IF EXISTS**: If specified, the statement succeeds silently when the plugin does not exist instead of returning an error.
23+
2224
plugin_name can be viewed through SHOW PLUGINS command
2325

2426
Only non-builtin plugins can be uninstalled.
@@ -30,3 +32,9 @@ Only non-builtin plugins can be uninstalled.
3032
```SQL
3133
UNINSTALL PLUGIN auditdemo;
3234
```
35+
36+
2. Uninstall a plugin if it exists:
37+
38+
```SQL
39+
UNINSTALL PLUGIN IF EXISTS auditdemo;
40+
```

‎fe/fe-core/src/main/java/com/starrocks/plugin/PluginMgr.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,10 @@ public PluginInfo installPlugin(InstallPluginStmt stmt) throws IOException, Star
143143
}
144144

145145
if (checkDynamicPluginNameExist(info.getName())) {
146+
if (stmt.isIfNotExists()) {
147+
LOG.info("install plugin[{}] which already exists", info.getName());
148+
return info;
149+
}
146150
throw new StarRocksException("plugin " + info.getName() + " has already been installed.");
147151
}
148152

@@ -169,6 +173,13 @@ public PluginInfo installPlugin(InstallPluginStmt stmt) throws IOException, Star
169173

170174
public void uninstallPluginFromStmt(UninstallPluginStmt stmt) throws IOException, StarRocksException {
171175
String pluginName = stmt.getPluginName();
176+
if (!checkDynamicPluginNameExist(pluginName)) {
177+
if (stmt.isIfExists()) {
178+
LOG.info("uninstall plugin[{}] which does not exist", pluginName);
179+
return;
180+
}
181+
throw new DdlException("Plugin " + pluginName + " does not exist");
182+
}
172183
int typeId = uninstallPlugin(pluginName);
173184

174185
GlobalStateMgr.getCurrentState().getEditLog().logUninstallPlugin(new UninstallPluginLog(pluginName), wal -> {

‎fe/fe-core/src/main/java/com/starrocks/sql/parser/AstBuilder.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4646,14 +4646,16 @@ public ParseNode visitShowExportStatement(com.starrocks.sql.parser.StarRocksPars
46464646
public ParseNode visitInstallPluginStatement(com.starrocks.sql.parser.StarRocksParser.InstallPluginStatementContext context) {
46474647
String pluginPath = ((Identifier) visit(context.identifierOrString())).getValue();
46484648
Map<String, String> properties = getCaseSensitiveProperties(context.properties());
4649-
return new InstallPluginStmt(pluginPath, properties, createPos(context));
4649+
boolean ifNotExists = context.IF() != null;
4650+
return new InstallPluginStmt(pluginPath, properties, ifNotExists, createPos(context));
46504651
}
46514652

46524653
@Override
46534654
public ParseNode visitUninstallPluginStatement(
46544655
com.starrocks.sql.parser.StarRocksParser.UninstallPluginStatementContext context) {
46554656
String pluginPath = ((Identifier) visit(context.identifierOrString())).getValue();
4656-
return new UninstallPluginStmt(pluginPath, createPos(context));
4657+
boolean ifExists = context.IF() != null;
4658+
return new UninstallPluginStmt(pluginPath, ifExists, createPos(context));
46574659
}
46584660

46594661
// ------------------------------------------------- File Statement ----------------------------------------------------------

‎fe/fe-core/src/test/java/com/starrocks/analysis/InstallPluginStmtTest.java‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,13 @@
2222
import com.starrocks.plugin.DynamicPluginLoader;
2323
import com.starrocks.qe.ConnectContext;
2424
import com.starrocks.sql.ast.InstallPluginStmt;
25+
import com.starrocks.sql.ast.StatementBase;
26+
import com.starrocks.sql.ast.UninstallPluginStmt;
27+
import com.starrocks.sql.parser.SqlParser;
2528
import org.junit.jupiter.api.Assertions;
2629
import org.junit.jupiter.api.Test;
2730

31+
import java.util.List;
2832
import java.util.Map;
2933

3034
public class InstallPluginStmtTest {
@@ -38,4 +42,51 @@ public void testNormal() throws StarRocksException {
3842
stmt.getProperties().get(DynamicPluginLoader.MD5SUM_KEY));
3943
Assertions.assertEquals("http://test/test.zip", stmt.getPluginPath());
4044
}
45+
46+
@Test
47+
public void testInstallPluginIfNotExistsParsedTrue() {
48+
String sql = "INSTALL PLUGIN IF NOT EXISTS FROM '/path/to/plugin.zip'";
49+
List<StatementBase> stmts = SqlParser.parse(sql, 32);
50+
InstallPluginStmt stmt = (InstallPluginStmt) stmts.get(0);
51+
Assertions.assertTrue(stmt.isIfNotExists());
52+
}
53+
54+
@Test
55+
public void testInstallPluginWithoutIfNotExistsDefaultsFalse() {
56+
String sql = "INSTALL PLUGIN FROM '/path/to/plugin.zip'";
57+
List<StatementBase> stmts = SqlParser.parse(sql, 32);
58+
InstallPluginStmt stmt = (InstallPluginStmt) stmts.get(0);
59+
Assertions.assertFalse(stmt.isIfNotExists());
60+
}
61+
62+
@Test
63+
public void testUninstallPluginIfExistsParsedTrue() {
64+
String sql = "UNINSTALL PLUGIN IF EXISTS my_plugin";
65+
List<StatementBase> stmts = SqlParser.parse(sql, 32);
66+
UninstallPluginStmt stmt = (UninstallPluginStmt) stmts.get(0);
67+
Assertions.assertTrue(stmt.isIfExists());
68+
}
69+
70+
@Test
71+
public void testUninstallPluginWithoutIfExistsDefaultsFalse() {
72+
String sql = "UNINSTALL PLUGIN my_plugin";
73+
List<StatementBase> stmts = SqlParser.parse(sql, 32);
74+
UninstallPluginStmt stmt = (UninstallPluginStmt) stmts.get(0);
75+
Assertions.assertFalse(stmt.isIfExists());
76+
}
77+
78+
@Test
79+
public void testInstallPluginConstructorDefaultsIfNotExistsFalse() {
80+
Map<String, String> props = Maps.newHashMap();
81+
InstallPluginStmt stmt = new InstallPluginStmt("http://test/plugin.zip", props);
82+
Assertions.assertFalse(stmt.isIfNotExists());
83+
Assertions.assertEquals("http://test/plugin.zip", stmt.getPluginPath());
84+
}
85+
86+
@Test
87+
public void testUninstallPluginConstructorDefaultsIfExistsFalse() {
88+
UninstallPluginStmt stmt = new UninstallPluginStmt("my_plugin");
89+
Assertions.assertFalse(stmt.isIfExists());
90+
Assertions.assertEquals("my_plugin", stmt.getPluginName());
91+
}
4192
}

‎fe/fe-core/src/test/java/com/starrocks/plugin/PluginMgrEditLogTest.java‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import com.starrocks.server.GlobalStateMgr;
2424
import com.starrocks.sql.ast.InstallPluginStmt;
2525
import com.starrocks.sql.ast.UninstallPluginStmt;
26+
import com.starrocks.sql.parser.NodePosition;
2627
import com.starrocks.utframe.UtFrameUtils;
2728
import mockit.Mock;
2829
import mockit.MockUp;
@@ -293,5 +294,43 @@ public void testUninstallPluginFromStmtMethodPluginNotExists() throws Exception
293294
});
294295
Assertions.assertTrue(exception.getMessage().contains("does not exist"));
295296
}
297+
298+
@Test
299+
public void testInstallPluginIfNotExistsWhenAlreadyInstalled() throws Exception {
300+
// Pre-register the plugin so it already exists
301+
PluginInfo pluginInfo = createMockPluginInfo();
302+
masterPluginMgr.replayLoadDynamicPlugin(pluginInfo);
303+
Assertions.assertTrue(pluginExists(masterPluginMgr, TEST_PLUGIN_NAME, PluginInfo.PluginType.AUDIT));
304+
305+
// Build stmt with IF NOT EXISTS flag set
306+
Map<String, String> properties = Maps.newHashMap();
307+
InstallPluginStmt stmt = new InstallPluginStmt("http://test/test.zip", properties, true, NodePosition.ZERO);
308+
309+
new MockUp<DynamicPluginLoader>() {
310+
@Mock
311+
public PluginInfo getPluginInfo() throws IOException {
312+
return pluginInfo;
313+
}
314+
};
315+
316+
// Should return the existing plugin info — no exception, no re-install
317+
PluginInfo result = masterPluginMgr.installPlugin(stmt);
318+
Assertions.assertSame(pluginInfo, result);
319+
320+
// Plugin should still be registered (untouched)
321+
Assertions.assertTrue(pluginExists(masterPluginMgr, TEST_PLUGIN_NAME, PluginInfo.PluginType.AUDIT));
322+
}
323+
324+
@Test
325+
public void testUninstallPluginIfExistsWhenNotInstalled() throws Exception {
326+
String nonExistentPlugin = "non_existent_plugin_ifexists";
327+
Assertions.assertFalse(pluginExists(masterPluginMgr, nonExistentPlugin, PluginInfo.PluginType.AUDIT));
328+
329+
// Build stmt with IF EXISTS flag set
330+
UninstallPluginStmt stmt = new UninstallPluginStmt(nonExistentPlugin, true, NodePosition.ZERO);
331+
332+
// Should return silently — no exception
333+
Assertions.assertDoesNotThrow(() -> masterPluginMgr.uninstallPluginFromStmt(stmt));
334+
}
296335
}
297336

‎fe/fe-core/src/test/java/com/starrocks/plugin/PluginMgrTest.java‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,20 +34,32 @@
3434

3535
package com.starrocks.plugin;
3636

37+
import com.google.common.collect.Maps;
3738
import com.starrocks.common.Config;
3839
import com.starrocks.common.util.DigitalVersion;
40+
import com.starrocks.persist.EditLog;
41+
import com.starrocks.persist.UninstallPluginLog;
42+
import com.starrocks.persist.WALApplier;
3943
import com.starrocks.plugin.PluginInfo.PluginType;
4044
import com.starrocks.server.GlobalStateMgr;
45+
import com.starrocks.sql.ast.InstallPluginStmt;
46+
import com.starrocks.sql.ast.UninstallPluginStmt;
47+
import com.starrocks.sql.parser.NodePosition;
4148
import com.starrocks.utframe.UtFrameUtils;
49+
import mockit.Mock;
50+
import mockit.MockUp;
4251
import org.apache.commons.io.FileUtils;
4352
import org.junit.jupiter.api.BeforeAll;
4453
import org.junit.jupiter.api.BeforeEach;
4554
import org.junit.jupiter.api.Test;
4655

4756
import java.io.IOException;
4857
import java.nio.file.Files;
58+
import java.util.Map;
4959

60+
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
5061
import static org.junit.jupiter.api.Assertions.assertFalse;
62+
import static org.junit.jupiter.api.Assertions.assertSame;
5163
import static org.junit.jupiter.api.Assertions.assertTrue;
5264

5365
public class PluginMgrTest {
@@ -89,4 +101,53 @@ public void testLoadPluginFail() {
89101
assert false;
90102
}
91103
}
104+
105+
@Test
106+
public void testInstallPluginIfNotExistsIdempotent() throws Exception {
107+
String pluginName = "idempotent_install_plugin";
108+
PluginInfo pluginInfo = new PluginInfo(pluginName, PluginType.AUDIT, "test");
109+
110+
PluginMgr pluginMgr = new PluginMgr();
111+
pluginMgr.replayLoadDynamicPlugin(pluginInfo);
112+
113+
new MockUp<DynamicPluginLoader>() {
114+
@Mock
115+
public PluginInfo getPluginInfo() throws IOException {
116+
return pluginInfo;
117+
}
118+
};
119+
120+
Map<String, String> props = Maps.newHashMap();
121+
InstallPluginStmt stmt = new InstallPluginStmt("http://dummy/test.zip", props, true, NodePosition.ZERO);
122+
PluginInfo result = pluginMgr.installPlugin(stmt);
123+
assertSame(pluginInfo, result);
124+
}
125+
126+
@Test
127+
public void testUninstallPluginIfExistsIdempotent() {
128+
PluginMgr pluginMgr = new PluginMgr();
129+
UninstallPluginStmt stmt = new UninstallPluginStmt("nonexistent_plugin", true, NodePosition.ZERO);
130+
assertDoesNotThrow(() -> pluginMgr.uninstallPluginFromStmt(stmt));
131+
}
132+
133+
@Test
134+
public void testUninstallPluginFromStmtNormal() throws Exception {
135+
String pluginName = "normal_uninstall_plugin";
136+
PluginInfo pluginInfo = new PluginInfo(pluginName, PluginType.AUDIT, "test");
137+
138+
PluginMgr pluginMgr = new PluginMgr();
139+
pluginMgr.replayLoadDynamicPlugin(pluginInfo);
140+
141+
new MockUp<EditLog>() {
142+
@Mock
143+
public void logUninstallPlugin(UninstallPluginLog log, WALApplier walApplier) {
144+
walApplier.apply(log);
145+
}
146+
};
147+
148+
UninstallPluginStmt stmt = new UninstallPluginStmt(pluginName);
149+
pluginMgr.uninstallPluginFromStmt(stmt);
150+
assertFalse(pluginMgr.getAllDynamicPluginInfo()
151+
.stream().anyMatch(p -> p.getName().equals(pluginName)));
152+
}
92153
}

‎fe/fe-grammar/src/main/antlr/com/starrocks/grammar/StarRocks.g4‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2191,11 +2191,11 @@ showExportStatement
21912191
// ------------------------------------------- Plugin Statement --------------------------------------------------------
21922192

21932193
installPluginStatement
2194-
: INSTALL PLUGIN FROM identifierOrString properties?
2194+
: INSTALL PLUGIN (IF NOT EXISTS)? FROM identifierOrString properties?
21952195
;
21962196

21972197
uninstallPluginStatement
2198-
: UNINSTALL PLUGIN identifierOrString
2198+
: UNINSTALL PLUGIN (IF EXISTS)? identifierOrString
21992199
;
22002200

22012201
// ------------------------------------------- File Statement ----------------------------------------------------------

‎fe/fe-parser/src/main/java/com/starrocks/sql/ast/InstallPluginStmt.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,15 +23,21 @@ public class InstallPluginStmt extends DdlStmt {
2323

2424
private final String pluginPath;
2525
private final Map<String, String> properties;
26+
private final boolean ifNotExists;
2627

2728
public InstallPluginStmt(String pluginPath, Map<String, String> properties) {
2829
this(pluginPath, properties, NodePosition.ZERO);
2930
}
3031

3132
public InstallPluginStmt(String pluginPath, Map<String, String> properties, NodePosition pos) {
33+
this(pluginPath, properties, false, pos);
34+
}
35+
36+
public InstallPluginStmt(String pluginPath, Map<String, String> properties, boolean ifNotExists, NodePosition pos) {
3237
super(pos);
3338
this.pluginPath = pluginPath;
3439
this.properties = properties;
40+
this.ifNotExists = ifNotExists;
3541
}
3642

3743
public String getPluginPath() {
@@ -42,6 +48,10 @@ public Map<String, String> getProperties() {
4248
return properties;
4349
}
4450

51+
public boolean isIfNotExists() {
52+
return ifNotExists;
53+
}
54+
4555
@Override
4656
public <R, C> R accept(AstVisitor<R, C> visitor, C context) {
4757
return visitor.visitInstallPluginStatement(this, context);

‎fe/fe-parser/src/main/java/com/starrocks/sql/ast/UninstallPluginStmt.java‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,20 +20,30 @@
2020
public class UninstallPluginStmt extends DdlStmt {
2121

2222
private final String pluginName;
23+
private final boolean ifExists;
2324

2425
public UninstallPluginStmt(String pluginName) {
25-
this(pluginName, NodePosition.ZERO);
26+
this(pluginName, false, NodePosition.ZERO);
2627
}
2728

2829
public UninstallPluginStmt(String pluginName, NodePosition pos) {
30+
this(pluginName, false, pos);
31+
}
32+
33+
public UninstallPluginStmt(String pluginName, boolean ifExists, NodePosition pos) {
2934
super(pos);
3035
this.pluginName = pluginName;
36+
this.ifExists = ifExists;
3137
}
3238

3339
public String getPluginName() {
3440
return pluginName;
3541
}
3642

43+
public boolean isIfExists() {
44+
return ifExists;
45+
}
46+
3747
@Override
3848
public <R, C> R accept(AstVisitor<R, C> visitor, C context) {
3949
return visitor.visitUninstallPluginStatement(this, context);

0 commit comments

Comments
 (0)