-
Notifications
You must be signed in to change notification settings - Fork 6
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
Fix fullscan index issue on tasks health #64
Fix fullscan index issue on tasks health #64
Conversation
6b33d9f
to
b64f662
Compare
src/main/scala/mesosphere/marathon/core/health/impl/HealthIndex.scala
Outdated
Show resolved
Hide resolved
src/main/scala/mesosphere/marathon/core/health/impl/HealthIndex.scala
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OPT: Overall, should we have a test for theses classes? even if those are simple tests?
src/main/scala/mesosphere/marathon/core/health/impl/HealthIndex.scala
Outdated
Show resolved
Hide resolved
src/main/scala/mesosphere/marathon/core/health/impl/HealthIndex.scala
Outdated
Show resolved
Hide resolved
src/main/scala/mesosphere/marathon/core/health/impl/HealthCheckActor.scala
Outdated
Show resolved
Hide resolved
cd6af78
to
8609866
Compare
src/main/scala/mesosphere/marathon/core/health/impl/HealthCheckActor.scala
Outdated
Show resolved
Hide resolved
As it was very slow with several thousands of tasks/instances, this uses two indexes instead.
8609866
to
6017a1d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, I'll do a non-reg test and also will validate in preprod before merging
As it was very slow with several thousands of tasks/instances, this uses two indexes instead.