[GOBBLIN-2016] Add eventTime to DagAction reminder key

[umustafi]

Dear Gobblin maintainers,

Please accept this PR. I understand that it will not be reviewed until I have checked off all the steps below!

JIRA

• My PR addresses the following Gobblin JIRA issues and references them in the PR title. For example, "[GOBBLIN-XXX] My Gobblin PR"
  • https://issues.apache.org/jira/browse/GOBBLIN-2106

Description

• Here are some details about my PR, including screenshots (if applicable):
  Handles the issue of identical KILL DagActions caused by multiple kill endpoint calls for the same FG+FN+flowExecId.

Adds eventTime to the key of the dagActionReminderScheduler to ensure unique keys for multiple instances of the same action on the same flow execution that originate more than 'epsilon' apart. MultiActiveLeaseArbiter uses the eventTime to distinguish these distinct occurrences of the same action. This is necessary to prevent insertion failures due to previous reminders.

Applicable only for KILL and RESUME actions; duplication for other actions is an error.

Tests

• My PR adds the following unit tests OR does not need testing for this extremely good reason:

• updates existing scheduler unit tests to check for eventTimeMillis in key

• also checks multiple flow execution deadlines can be added to scheduler at once

Commits

• My commits all reference JIRA issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "How to write a good git commit message":
  1. Subject is separated from body by a blank line
  2. Subject is limited to 50 characters
  3. Subject does not end with a period
  4. Subject uses the imperative mood ("add", not "adding")
  5. Body wraps at 72 characters
  6. Body explains "what" and "why", not "how"

[umustafi]

there's no event time in dagActionStore so cannot remove job from scheduler & this will be addressed in future pr to handle race condition between creating and deleting trigger

[phet]

this doesn't capture the nuance of why incorporating the flowExecId into the key (as already being done) is insufficient and an event timestamp is needed in addition.

please explain... (doesn't it have to do w/ event consolidation?)

[umustafi]

I believe there is a valid scenario to have multiple kill (or resume) dagActions on the same flow execution that are requested far enough apart (let's say 30 sec apart) to be considered unique events and we were not able to handle storage of reminders for the same actionType on the same execution. If epsilon is 20 sec and the requests are 30 sec apart they're distinct, but if the reminder is 90 sec then there is overlap for storing these reminders in the scheduler.

[phet]

when you say "deadline period", that's not related to job-start or flow-completion deadlines, is it? do you mean "reminder period" (approx. the 'linger' setting)?

also, this is when multiple instances of the same action on the same flow execution originate more than 'epsilon' apart, right? and it's applicable to only KILL and RESUME, not other types, correct?

[umustafi]

it should be deadline or reminder period and more likely the former will actually cause the issue bc the SLA deadline (ie: job complete within 2 hours) is stored in this scheduler and will conflict with another SLA deadline reminder (job complete within 2 hours) stored in the Scheduler. The same conflict can also happen with the reminder period.

The latter is precisely correct, yes.

[phet]

good work!