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.

Sign up for GitHub

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

Jump to bottom

Makes sure the workspace activity checker reconciles the workspaces with their true statuses. #14574

Merged
merged 7 commits into from
Sep 20, 2019
Merged

Makes sure the workspace activity checker reconciles the workspaces with their true statuses. #14574

merged 7 commits into from
Sep 20, 2019

Conversation

metlos
Copy link
Contributor

@metlos metlos commented Sep 17, 2019

What does this PR do?

Because it seems like under heavy load we either loose or don't register workspace state transitions in the workspace activity checker, this PR makes sure that we go through all the current activity records and reconcile them with the actual workspace status as registered by the workspace runtimes.

What issues does this PR fix or reference?

#14536

@metlos
Makes sure the workspace activity checker reconciles the workspaces
13573ce 
with their true statuses.

Signed-off-by: Lukas Krejci <lkrejci@redhat.com>
@che-bot che-bot added status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. kind/bug Outline of a bug - must adhere to the bug report template. labels Sep 17, 2019
@che-bot
Copy link
Contributor

che-bot commented Sep 17, 2019

E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has been successful:

  • build details
  • "che-server" docker image: maxura/che-server:14574

@skabashnyuk
Copy link
Contributor

skabashnyuk commented Sep 17, 2019 •
edited
Loading

I have some doubts that getting all records with activities in a single query is a good thing. Can we do that with paging? or can we change it to the records that have mismatched status with runtime table?

@che-bot
Copy link
Contributor

che-bot commented Sep 17, 2019

E2E tests of Eclipse Che Multiuser on OCP has failed:

@che-bot
Copy link
Contributor

che-bot commented Sep 18, 2019

E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has been successful:

  • build details
  • "che-server" docker image: maxura/che-server:14574

@che-bot
Copy link
Contributor

che-bot commented Sep 18, 2019

E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has been successful:

  • build details
  • "che-server" docker image: maxura/che-server:14574

@che-bot
Copy link
Contributor

che-bot commented Sep 18, 2019

E2E tests of Eclipse Che Multiuser on OCP has been successful:

@che-bot
Copy link
Contributor

che-bot commented Sep 19, 2019

E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has been successful:

  • build details
  • "che-server" docker image: maxura/che-server:14574

@che-bot
Copy link
Contributor

che-bot commented Sep 19, 2019

E2E tests of Eclipse Che Multiuser on OCP has been successful:

@metlos
Paginate the WorkspaceActivity.getAll() and use the paged version in
62e11c8 
the activity checker.

Signed-off-by: Lukas Krejci <lkrejci@redhat.com>
@metlos metlos force-pushed the bug/14536-inconsistency-in-ws-activity branch from 46744c0 to 62e11c8 Compare September 19, 2019 14:16
@metlos
The (unused) InmemoryWorkspaceActivityDao implementation of getAll()
b777be9 
was slightly wrong.

Signed-off-by: Lukas Krejci <lkrejci@redhat.com>
@che-bot
Copy link
Contributor

che-bot commented Sep 19, 2019

E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has failed:

@che-bot
Copy link
Contributor

che-bot commented Sep 19, 2019

E2E tests of Eclipse Che Multiuser on OCP has failed:

sleshchenko
sleshchenko approved these changes Sep 19, 2019
View reviewed changes
Copy link
Member

@sleshchenko sleshchenko left a comment

Choose a reason for hiding this comment

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

LGTM

...-activity/src/main/java/org/eclipse/che/api/workspace/activity/WorkspaceActivityChecker.java Show resolved Hide resolved
...-activity/src/main/java/org/eclipse/che/api/workspace/activity/WorkspaceActivityChecker.java Show resolved Hide resolved
@che-bot
Copy link
Contributor

che-bot commented Sep 19, 2019

E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has failed:

@metlos
Fix the count query. Seems like JPA is not SQL after all... :)
9b4f683 
Signed-off-by: Lukas Krejci <lkrejci@redhat.com>
@che-bot
Copy link
Contributor

che-bot commented Sep 19, 2019

E2E tests of Eclipse Che Multiuser on OCP has failed:

@che-bot
Copy link
Contributor

che-bot commented Sep 19, 2019

E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has been successful:

  • build details
  • "che-server" docker image: maxura/che-server:14574

@metlos
fix the tests
e33a177 
Signed-off-by: Lukas Krejci <lkrejci@redhat.com>
@metlos
Introduce a separate config variable to specify the period of the
75d5194 
workspace activity cleanup job.

Signed-off-by: Lukas Krejci <lkrejci@redhat.com>
@che-bot
Copy link
Contributor

che-bot commented Sep 20, 2019

E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has been successful:

  • build details
  • "che-server" docker image: maxura/che-server:14574

@che-bot
Copy link
Contributor

che-bot commented Sep 20, 2019

E2E tests of Eclipse Che Multiuser on OCP has been successful:

@metlos
Have a separate initial delay for activity clean up.
6ca7e6d 
Signed-off-by: Lukas Krejci <lkrejci@redhat.com>
@che-bot
Copy link
Contributor

che-bot commented Sep 20, 2019

E2E Happy path tests of Eclipse Che Single User on K8S (minikube v1.1.1) has been successful:

  • build details
  • "che-server" docker image: maxura/che-server:14574

@metlos
Copy link
Contributor Author

metlos commented Sep 20, 2019

ci-build

@che-bot
Copy link
Contributor

che-bot commented Sep 20, 2019

E2E tests of Eclipse Che Multiuser on OCP has been successful:

@metlos metlos merged commit 159ccc6 into eclipse-che:master Sep 20, 2019
@che-bot che-bot removed the status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. label Sep 20, 2019
@che-bot che-bot added this to the 7.2.0 milestone Sep 20, 2019
@metlos metlos mentioned this pull request Sep 20, 2019
Closed
19 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sleshchenko sleshchenko sleshchenko approved these changes

@skabashnyuk skabashnyuk skabashnyuk approved these changes

@l0rd l0rd Awaiting requested review from l0rd

@mshaposhnik mshaposhnik Awaiting requested review from mshaposhnik

@nickboldt nickboldt Awaiting requested review from nickboldt

@rhopp rhopp Awaiting requested review from rhopp

@amisevsk amisevsk Awaiting requested review from amisevsk

@ibuziuk ibuziuk Awaiting requested review from ibuziuk

Assignees
No one assigned
Labels
kind/bug Outline of a bug - must adhere to the bug report template.
Projects
None yet
Milestone
7.2.0
Development

Successfully merging this pull request may close these issues.
4 participants
@metlos @che-bot @skabashnyuk @sleshchenko