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

#795 - CCTray Perf Fix #877

Merged
merged 23 commits into from Feb 16, 2015

Conversation

Projects
None yet
4 participants
@arvindsv
Copy link
Member

commented Feb 5, 2015

This is a parallel implementation of cctray, available at /go/new_cctray.xml (for now) /go/cctray.xml (after #1464 was merged). It reuses much of the existing code, but implements caching in a better way (as mentioned in the mail referenced in #795).

Still need to verify the performance benefits of this. But, this can be accepted if someone has the time to review it, since it is implicitly feature toggled (since the URL is new).

arvindsv added some commits Jan 29, 2015

#795 - Initial CCTray fix commit.
Since job, stage and config can change simultaneously, put in a queue to multiplex all their calls
onto their own handlers, running on a single thread. That way, the cache does not need to have very
strong concurrency semantics. It'll need to be a single-thread-write and multi-thread-read.
#795 - Job status change handled.
* Move relevant code from CcTrayStatus#jobStatusChanged and dependencies to CcTrayJobStatusChangeHandler.
#795 - Stage status change handled.
* Move relevant code from CcTrayStatus#stageStatusChanged and dependencies to CcTrayStageStatusChangeHandler.
#795 - Change CcTrayJobStatusChangeHandler interface.
Change the interface of CcTrayJobStatusChangeHandler so that it can be
used by the config change handler too, since it needs to convert Job
instances to ProjectStatus.
#795 - Change CcTrayStageStatusChangeHandler interface.
Change the interface of CcTrayStageStatusChangeHandler so that it can be
used by the config change handler too, since it needs to convert Job
instances to ProjectStatus.
#795 - Config change handled.
* Stages already in cache are used.
* If they're not in the cache, then the DB is queried.
* If a stage in not in DB as well (never run), then a dummy stage status is used.
* Handles new jobs in config (converted to dummy state) and removed jobs (removes from cache).
#795 - Handle breakers.
Breakers are calculated as before. This is almost a full migration from
CcTrayStatus#computeBreakersIfStageFailed to CcTrayBreakersCalculator.
#795 - Handle loading of stage info from DB.
Almost a migration from old CcTrayStatusService#initializeMissingStages method
to CcTrayStageStatusLoader.
#795 - Switch from StageService to StageDao.
CcTrayStageStatusLoader needs to load a stage from DB. But, it causes a circular dependency
if it uses StageService, because StageService needs all StageStatusListener objects during
construction. CcTrayActivityListener is a StageStatusListener, which depends on
CcTrayStageStatusChangeHandler, which is a dependency of CcTrayStageStatusLoader (circular).
#795 - Decide which users can view CCTray.
Flattens out group and superadmin user and role permissions into a map of
pipeline groups to the set of users who can view them.
#795 - Update view permissions for every project.
Depending on the view permissions of the pipeline group, update each CcTray project
status with the viewers, so that they need not be checked every time.
#795 - Provide ordered list of projects from cache.
Prevents config from having to be iterated on, to get projects.
#795 - Create CcTray XML representation for serving.
* Consider user permissions (who has access to a pipeline).
* Generate XML and cache parts when possible.
#795 - Register CcTrayListener for config change.
Unlike the job and stage status change listeners, the config change listener requires
explicit registration. Hook up CcTrayListener to ApplicationInitializer, to guarantee
order of registration.
#795 - Fix issue of cache update on job/stage change.
When a job or stage changes, the cache is updated with the new status. But, the orderedEntries list
was not updated, since it was maintained externally. Now, use a LinkedHashMap to to maintain order.
#795 - Do not lose view permissions when job changes.
When a job or stage status changes, the existing view permissions in cache should be
preserved. That information changes only when config changes.
#795 - Start queue processor during initialize.
* The CcTrayActivityListener queue processor is now started during initialize.
  Otherwise, it won't process anything.

* Also, mark the orderedEntries field in CcTrayCache as volatile, since it is
  accessed by multiple threads and changed by one.
#795 - Hook up to UI.
* Provide route in Rails to new_cctray.xml (temporary route name).
* Use context and request paths to decide on correct prefix (as was done
  with old CCTray requests).
#000 - Fix spec which randomly fails.
A value passed to localize was nil. Set it to a valid value in the spec.
@arvindsv

This comment has been minimized.

Copy link
Member Author

commented Feb 5, 2015

Personal build is green: go/pipelines/value_stream_map/personal-arvind/7 on 05.

arvindsv added some commits Feb 5, 2015

#795 - Move a few methods around.
Within the same class. public methods up. private methods down.
#795 - Make new CCTray call 401 on auth fail.
Without this change, it redirects to login page. Compare:
curl -I 'http://localhost:8153/go/cctray.xml'
curl -I 'http://localhost:8153/go/new_cctray.xml'
@arvindsv

This comment has been minimized.

Copy link
Member Author

commented Feb 10, 2015

Some performance numbers are here.

jyotisingh added a commit that referenced this pull request Feb 16, 2015

@jyotisingh jyotisingh merged commit 4af3124 into gocd:master Feb 16, 2015

@jyotisingh jyotisingh removed their assignment Feb 24, 2015

@arvindsv arvindsv deleted the arvindsv:795_cctray_perf_fix branch Jun 23, 2015

@mrmanc

This comment has been minimized.

Copy link
Contributor

commented Dec 16, 2015

Hi… just got here from the release notes for 15.1 and thought I’d mention that the URL mentioned in the first message is no longer correct—would it be worth editing it to indicate that the url is not /go/new_cctray.xml?

@arvindsv

This comment has been minimized.

Copy link
Member Author

commented Dec 16, 2015

Done.

@ketan ketan modified the milestone: Old Milestone Feb 1, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.