SEPURI-SAI-KRISHNA commented on PR #18544:
URL:
https://github.com/apache/dolphinscheduler/pull/18544#issuecomment-5261943384
@SbloodyS Thanks, that definition helps, and I've now made the query follow
it (below).
On the fixture first: it contains one `flag = YES` row and one `flag = NO`
row, i.e. exactly one
valid instance plus one historical instance, which is the state
`RetryTaskInstanceFactory` leaves
behind. By your own definition that's clean data. The only unusual thing
about it is that the two
rows carry the same `end_time`, and that part isn't written by the engine,
it's what the column does
to two different timestamps: on MySQL `t_ds_task_instance.end_time` is a
`datetime` with no
fractional seconds, and MySQL rounds a fractional value to whole seconds on
insert (5.6.4+). Two
attempts a few hundred ms apart can therefore be stored with an identical
`end_time`.
## Steps to reproduce
Metadata DB must be MySQL (see the note at the end for why).
1. Workflow **A**, one SHELL task `t1` whose first attempt fails and whose
retry succeeds, so that
**A finishes SUCCESS** (required: `DependentExecute` only evaluates an
upstream instance that
succeeded):
```bash
test -f /tmp/ds18543.flag && exit 0
touch /tmp/ds18543.flag
exit 1
```
Set **Number of failed retries = 1** and **Failed retry interval = 0**,
so the retry starts
immediately and both attempts land in the same second.
2. Run **A**. It ends `SUCCESS` with two task instances for `t1`.
3. Check what was stored:
```sql
select id, task_code, state, flag, end_time
from t_ds_task_instance
where workflow_instance_id = <A_instance_id>
order by id;
```
The failed attempt is `flag = 0`, the retry is `flag = 1`, and both rows
show the **same**
`end_time` whenever the two attempts round into the same stored second.
4. Workflow **B** with a DEPENDENT task on workflow **A** with **ALL tasks**
selected
(`DEPENDENT_ALL_TASK_CODE`). Run **B**.
5. The dependent check fails in the master with
```
java.lang.IllegalStateException: Duplicate key <taskCode> (attempted
merging values ... and ...)
```
instead of resolving the dependency.
Being straight about it: step 3 is timing dependent. If the two attempts
happen to straddle a second
boundary you get two distinct `end_time` values and have to run it again
(`rm /tmp/ds18543.flag`
between runs). That is exactly why the unit test constructs the stored state
directly instead of
racing the clock. What is *not* timing dependent is the DAO behaviour once
two such rows exist, and
that is what the test pins down.
I also want to be upfront that I found this by reading the query rather than
from a production
incident, and that it is mainly a MySQL exposure: on PostgreSQL `end_time`
is a `timestamp` with
microsecond precision, so the collision is far less likely there.
## What I changed
Since `flag = YES` is the valid instance, the query should say so, so I
added it:
```sql
select max(id) as max_id
from t_ds_task_instance
where 1=1
and workflow_instance_id = #{workflowInstanceId}
and state != 8
and flag = 1
and end_time is not null
...
group by task_code
```
`flag = 1` enforces the invariant you described, and `max(id)` keeps the
result deterministic if two
valid rows ever do coexist. Full `dolphinscheduler-dao` suite passes, 284
tests.
If you would rather have only `and flag = 1` and keep `max(end_time)` as the
join key, I'm happy to
push that instead, just say the word.
--
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]
