khandelwal-prateek commented on code in PR #4136:
URL: https://github.com/apache/gobblin/pull/4136#discussion_r2306072649
##########
gobblin-restli/gobblin-flow-config-service/gobblin-flow-config-service-server/src/main/java/org/apache/gobblin/service/modules/restli/FlowConfigsV2ResourceHandler.java:
##########
@@ -248,8 +251,22 @@ public CreateKVResponse<ComplexResourceKey<FlowId,
FlowStatusId>, FlowConfig> cr
// Return conflict and take no action if flowSpec has already been created
if (this.flowCatalog.exists(flowSpec.getUri())) {
log.warn("FlowSpec with URI {} already exists, no action will be taken",
flowSpec.getUri());
- return new CreateKVResponse<>(new
RestLiServiceException(HttpStatus.S_409_CONFLICT,
- "FlowSpec with URI " + flowSpec.getUri() + " already exists, no
action will be taken"));
+ try {
+ FlowSpec storedFlowSpec = this.flowCatalog.getSpecs(flowSpec.getUri());
+ if (!storedFlowSpec.isScheduled()) {
+ log.warn("FlowSpec Already Exists As Adhoc Flow with URI: " +
flowSpec.getUri());
+ if (!flowSpec.isScheduled()) {
+ flowSpecExistsForAdhocFlow.mark();
Review Comment:
there’s an expected case where a FlowSpec isn't deleted for an adhoc flow,
specifically when a new flow is triggered before the previous one is launched.
In that case, we throw a `TooSoonToRerunSameFlowException` from
`Orchestrator.onAddSpec`. We should exclude these scenarios, since these are
valid, and only flag cases where the FlowSpec wasn't deleted even after the DAG
was launched, which is unexpected.
This would mark the metric for both the scenarios and we would have
false/non-actionable alerts in such cases
--
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: dev-unsubscr...@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
