Skip to content

#795 - CCTray Perf Fix#877

Merged
jyotisingh merged 23 commits intogocd:masterfrom
arvindsv:795_cctray_perf_fix
Feb 16, 2015
Merged

#795 - CCTray Perf Fix#877
jyotisingh merged 23 commits intogocd:masterfrom
arvindsv:795_cctray_perf_fix

Conversation

@arvindsv
Copy link
Member

@arvindsv arvindsv 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).

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.
* Move relevant code from CcTrayStatus#jobStatusChanged and dependencies to CcTrayJobStatusChangeHandler.
* Move relevant code from CcTrayStatus#stageStatusChanged and dependencies to CcTrayStageStatusChangeHandler.
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.
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.
* 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).
Breakers are calculated as before. This is almost a full migration from
CcTrayStatus#computeBreakersIfStageFailed to CcTrayBreakersCalculator.
Almost a migration from old CcTrayStatusService#initializeMissingStages method
to CcTrayStageStatusLoader.
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).
Flattens out group and superadmin user and role permissions into a map of
pipeline groups to the set of users who can view them.
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.
Prevents config from having to be iterated on, to get projects.
* Consider user permissions (who has access to a pipeline).
* Generate XML and cache parts when possible.
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.
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.
When a job or stage status changes, the existing view permissions in cache should be
preserved. That information changes only when config changes.
* 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.
* 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).
A value passed to localize was nil. Set it to a valid value in the spec.
@arvindsv
Copy link
Member Author

arvindsv commented Feb 5, 2015

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

Within the same class. public methods up. private methods down.
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
Copy link
Member Author

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 795_cctray_perf_fix branch June 23, 2015 13:41
@mrmanc
Copy link
Contributor

mrmanc 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
Copy link
Member Author

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants