njnu-seafish commented on code in PR #18549:
URL:
https://github.com/apache/dolphinscheduler/pull/18549#discussion_r3904075058
##########
dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/AlertMapper.xml:
##########
@@ -51,6 +51,42 @@
having count(*) = 0
</insert>
+ <!-- MySQL -->
+ <insert id="insertTaskResultAlertIfAbsent" databaseId="mysql">
+ INSERT INTO t_ds_alert(sign, title, content, alert_status,
warning_type, log, alertgroup_id,
+ create_time, update_time, project_code,
workflow_definition_code,
+ workflow_instance_id, alert_type)
+ VALUES (#{alert.sign}, #{alert.title}, #{alert.content},
#{alert.alertStatus.code},
+ #{alert.warningType.code}, #{alert.log},
#{alert.alertGroupId}, #{alert.createTime},
+ #{alert.updateTime}, #{alert.projectCode},
#{alert.workflowDefinitionCode},
+ #{alert.workflowInstanceId}, #{alert.alertType.code})
+ ON DUPLICATE KEY UPDATE id = id
+ </insert>
+
+ <!-- H2 -->
+ <insert id="insertTaskResultAlertIfAbsent" databaseId="h2">
+ MERGE INTO t_ds_alert(sign, title, content, alert_status,
warning_type, log, alertgroup_id,
+ create_time, update_time, project_code, workflow_definition_code,
+ workflow_instance_id, alert_type)
+ KEY(sign, workflow_instance_id, alert_type)
+ VALUES (#{alert.sign}, #{alert.title}, #{alert.content},
#{alert.alertStatus.code},
+ #{alert.warningType.code}, #{alert.log}, #{alert.alertGroupId},
#{alert.createTime},
+ #{alert.updateTime}, #{alert.projectCode},
#{alert.workflowDefinitionCode},
+ #{alert.workflowInstanceId}, #{alert.alertType.code})
+ </insert>
+
+ <!-- PostgreSQL -->
+ <insert id="insertTaskResultAlertIfAbsent" databaseId="postgresql">
+ INSERT INTO t_ds_alert(sign, title, content, alert_status,
warning_type, log, alertgroup_id,
+ create_time, update_time, project_code,
workflow_definition_code,
+ workflow_instance_id, alert_type)
+ VALUES (#{alert.sign}, #{alert.title}, #{alert.content},
#{alert.alertStatus.code},
+ #{alert.warningType.code}, #{alert.log},
#{alert.alertGroupId}, #{alert.createTime},
+ #{alert.updateTime}, #{alert.projectCode},
#{alert.workflowDefinitionCode},
+ #{alert.workflowInstanceId}, #{alert.alertType.code})
+ ON CONFLICT (sign, workflow_instance_id, alert_type) DO NOTHING
+ </insert>
Review Comment:
> Don't use this writing method, which will lead to poor database
performance.
Replaced the dialect-specific upsert (ON DUPLICATE KEY UPDATE / MERGE INTO /
ON CONFLICT DO NOTHING) with a single dialect-neutral INSERT ... SELECT ...
WHERE NOT EXISTS statement.
This approach follows the same pattern already used by
insertAlertWhenServerCrash in the same mapper file, and avoids the following
performance concerns with upsert syntax:
**MySQL ON DUPLICATE KEY UPDATE:** triggers a unique-index probe and may
acquire gap/next-key locks even when the "update" is a no-op (id = id), adding
lock contention under concurrent access.
**H2 MERGE INTO:** performs a separate SELECT then conditional INSERT,
adding overhead on every call.
**PostgreSQL ON CONFLICT:** similarly incurs an extra index probe per insert.
The new SQL uses a single INSERT ... SELECT ... WHERE NOT EXISTS query. The
NOT EXISTS subquery short-circuits on the first matching row via the
uk_alert_dedup index, avoiding full-row scanning or locking overhead. This is
one SQL statement, no dialect branches, and no upsert-specific locking.
The uk_alert_dedup unique constraint is retained as a concurrent-safety net.
If a race condition causes two threads to pass NOT EXISTS simultaneously, the
constraint catches the duplicate insert.
The DAO layer catches DuplicateKeyException and returns 0, ensuring
idempotency without propagating exceptions to the caller.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
