You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dolphinscheduler.apache.org by GitBox <gi...@apache.org> on 2022/09/08 03:26:36 UTC
[GitHub] [dolphinscheduler] caishunfeng commented on a diff in pull request #11471: Fix insertOrUpdate plugin may failed due to concurrent operation
caishunfeng commented on code in PR #11471:
URL: https://github.com/apache/dolphinscheduler/pull/11471#discussion_r965467039
##########
dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/PluginDao.java:
##########
@@ -51,10 +57,21 @@ public int addOrUpdatePluginDefine(PluginDefine pluginDefine) {
PluginDefine currPluginDefine = pluginDefineMapper.queryByNameAndType(pluginDefine.getPluginName(), pluginDefine.getPluginType());
if (currPluginDefine == null) {
- if (pluginDefineMapper.insert(pluginDefine) == 1 && pluginDefine.getId() > 0) {
- return pluginDefine.getId();
+ try {
+ if (pluginDefineMapper.insert(pluginDefine) == 1) {
+ return pluginDefine.getId();
+ } else {
+ throw new TaskPluginException(String.format("Failed to insert plugin definition, pluginName: %s, pluginType: %s", pluginDefine.getPluginName(), pluginDefine.getPluginType()));
+ }
+ } catch (TaskPluginException ex) {
+ throw ex;
+ } catch (Exception ex) {
+ logger.info("Insert plugin definition error, there may already exist a plugin");
Review Comment:
```suggestion
logger.error("Insert plugin definition error, there may already exist a plugin", ex);
```
##########
dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/PluginDao.java:
##########
@@ -51,10 +57,21 @@ public int addOrUpdatePluginDefine(PluginDefine pluginDefine) {
PluginDefine currPluginDefine = pluginDefineMapper.queryByNameAndType(pluginDefine.getPluginName(), pluginDefine.getPluginType());
if (currPluginDefine == null) {
- if (pluginDefineMapper.insert(pluginDefine) == 1 && pluginDefine.getId() > 0) {
- return pluginDefine.getId();
+ try {
+ if (pluginDefineMapper.insert(pluginDefine) == 1) {
+ return pluginDefine.getId();
+ } else {
+ throw new TaskPluginException(String.format("Failed to insert plugin definition, pluginName: %s, pluginType: %s", pluginDefine.getPluginName(), pluginDefine.getPluginType()));
+ }
Review Comment:
```suggestion
}
throw new TaskPluginException(String.format("Failed to insert plugin definition, pluginName: %s, pluginType: %s", pluginDefine.getPluginName(), pluginDefine.getPluginType()));
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@dolphinscheduler.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org