[ 
https://issues.apache.org/jira/browse/GOBBLIN-1859?focusedWorklogId=872589&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-872589
 ]

ASF GitHub Bot logged work on GOBBLIN-1859:
-------------------------------------------

                Author: ASF GitHub Bot
            Created on: 24/Jul/23 16:27
            Start Date: 24/Jul/23 16:27
    Worklog Time Spent: 10m 
      Work Description: phet commented on code in PR #3721:
URL: https://github.com/apache/gobblin/pull/3721#discussion_r1272492670


##########
gobblin-runtime/src/main/java/org/apache/gobblin/runtime/api/MysqlMultiActiveLeaseArbiter.java:
##########
@@ -122,7 +122,8 @@ protected interface CheckedFunction<T, R> {
   // Need to define three separate statements to handle cases where row does 
not exist or has null values to check
   protected static final String 
CONDITIONALLY_ACQUIRE_LEASE_IF_NEW_ROW_STATEMENT = "INSERT INTO %s (flow_group, 
"
       + "flow_name, flow_execution_id, flow_action, event_timestamp, 
lease_acquisition_timestamp) "
-      + "VALUES (?, ?, ?, ?, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)";
+      + "SELECT ?, ?, ?, ?, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP WHERE NOT 
EXISTS (SELECT * FROM %s "
+      + WHERE_CLAUSE_TO_MATCH_KEY + ")";

Review Comment:
   IMO "where not exists" approaches obfuscation, especially in the midst of 
both the prepare statement `?` placeholders and the `%s` formats for the table 
name.  given the baseline cost of a remote DB call, an exception is not 
terribly expensive.  thus, I'd recommend `INSERT` w/o `IGNORE` and then to be 
prepared to catch it





Issue Time Tracking
-------------------

    Worklog Id:     (was: 872589)
    Time Spent: 50m  (was: 40m)

> Multi-active Unit Test for Multiple Participant
> -----------------------------------------------
>
>                 Key: GOBBLIN-1859
>                 URL: https://issues.apache.org/jira/browse/GOBBLIN-1859
>             Project: Apache Gobblin
>          Issue Type: Bug
>          Components: gobblin-service
>            Reporter: Urmi Mustafi
>            Assignee: Abhishek Tiwari
>            Priority: Major
>          Time Spent: 50m
>  Remaining Estimate: 0h
>
> In multi-active mode, multiple participants will be reading/writing to MySQL 
> table. We want to make sure each participant acts on the table in a manner 
> that takes into account that the state may have changed while the 
> MultiActiveLeaseArbiter is processing the result of a READ. This PR adds unit 
> tests that validate SQL Insertion statements are conditional upon the state 
> of the particular row corresponding to the flow action event being unchanged 
> from the read made by this participant. 



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to