Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use leader cache first for launching check, also check staging #2126

Merged
merged 2 commits into from
Aug 25, 2020

Conversation

ssalinas
Copy link
Member

Follow up to #2122 and #2123 , I think I was able to fix both issues:

  1. The previous zk check for !exists... was actually correct. However, I realized that some mesos executors do not utilize the task starting state at all, only task staging or task running. So I added both of those to the check
  2. This could be more efficient anyway since it should almost always run on the leading instance where the leader cache is already present. When running on the leader, this will now avoid those zk checks entirely and just use the list of history updates already in memory to determine what the most recent state is. This gives us a very simple comparison of == TASK_LAUNCHED to check, not having to worry about which states specific executors use

cc @relistan

@ssalinas ssalinas added the staging Merged to staging branch label Aug 25, 2020
Copy link
Contributor

@ajammala ajammala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚢

@ssalinas ssalinas merged commit efeddbb into master Aug 25, 2020
@ssalinas ssalinas deleted the reconcile_fix branch August 25, 2020 14:25
@relistan
Copy link
Contributor

relistan commented Sep 1, 2020

Aaah, this makes sense @ssalinas. After reading your comments on the other issue I had just come to the conclusion that it must have something to do with the behavior of your executor vs the regular Mesos Docker containerizer and our own executor. We will try this out when we next upgrade. 🍻

@ssalinas ssalinas added this to the 1.3.0 milestone Sep 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
staging Merged to staging branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants