Upgrade to Pro — share decks privately, control downloads, hide ads and more …

Refactoring Systems with Confidence

Refactoring Systems with Confidence

Over the last year and a half my team and I replaced a large subsystem in our application without (hardly) anyone noticing. I talked about that story, the tools and techniques we used, and what we learned along the way.

Scientist is open-source: https://github.com/github/scientist

Talk given at denver.rb and boulder.rb, December 2014.

Avatar for Nathan Witmer

Nathan Witmer

December 09, 2014
Tweet

More Decks by Nathan Witmer

Other Decks in Programming

Transcript

  1. SELECT rs.id as ris, 2 as ps from rs WHERE

    rs.wid = 123 UNION ALL SELECT rs.id as ris, 1 as ps from rs INNER JOIN ps ON rs.id = ps.rid WHERE ps.uid = 123 UNION ALL SELECT rs.id as ris, 2 as ps from rs INNER JOIN ts ON rs.oid = ts.oid INNER JOIN tms ON ts.id = tms.tid WHERE ts.n = 1 AND tms.uid = 123 UNION ALL SELECT rs.id as ris, GROUP_CONCAT(distinct ts.p) as ps from rs INNER JOIN tms rt ON rs.id = rt.rid INNER JOIN ts ON rt.tid = ts.id INNER JOIN tms ut ON ts.id = ut.tid WHERE ut.uid = 123 AND ts.n != 1 AND ts.p in (0,1,2) GROUP BY rs.id UNION ALL SELECT r.id as ri, 0 as p from rs JOIN us ON rs.wid = us.id JOIN ts ON ts.oid = us.id JOIN tms ON ts.id = tms.tid WHERE us.type = 0 AND ts.n = 1 AND tms.uid = 123 AND (rs.p = 0) AND rs.pid IS NOT NULL WHERE rs.id = ris
  2. –@rick, 2012 We could do worse than cleaning up the

    repo / permissioning mess, but it will take quite a bit of effort. It's part of the core domain, it's in the middle of the app, and it's going to eventually bite everyone who comes near it.
  3. –@defunkt, 2012 I'd love to see someone take a step

    back and define all the possible relationships a user can have to a repository, then wrap that up in a tasty API.
  4. User Repository Team read write Actor Action Subject User read

    Team Team write Repository User write Repository write
  5. user.can? :read, repository SELECT 1 FROM abilities WHERE actor_id =

    user_id AND actor_type = 'User' AND subject_id = repository_id AND subject_type = ‘Repository'
  6. class Repository < ActiveRecord::Base def pullable_by?(user) pullable_by_legacy?(user) end def pullable_by_legacy?(user)

    # old code end def pullable_by_abilities?(user) user.can? :read, self end end
  7. class Repository < ActiveRecord::Base include Scientist def pullable_by?(user) pullable_by_legacy?(user) end

    def pullable_by_legacy?(user) # old code end def pullable_by_abilities?(user) user.can? :read, self end end
  8. class Repository < ActiveRecord::Base include Scientist def pullable_by?(user) # Let’s

    do an experiment: science "repository.pullable_by" do |experiment| end end # ... end

  9. class Repository < ActiveRecord::Base include Scientist def pullable_by?(user) # Let’s

    do an experiment: science "repository.pullable_by" do |experiment| # Return this value no matter what experiment.use { pullable_by_legacy?(user) } end end # ... end
  10. class Repository < ActiveRecord::Base include Scientist def pullable_by?(user) # Let’s

    do an experiment: science "repository.pullable_by" do |experiment| # Return this value no matter what experiment.use { pullable_by_legacy?(user) } # Run the new code too, and compare the results experiment.try { pullable_by_abilities?(user) } end end end
  11. class Repository < ActiveRecord::Base include Scientist def pullable_by?(user) # Let's

    do an experiment science "repository.pullable_by" do |experiment| # Return this value no matter what experiment.use { pullable_by_legacy?(user) } # Run the new code too, and compare the results experiment.try { pullable_by_abilities?(user) } end end end
  12. class MyExperiment < ActiveRecord::Base include Scientist::Experiment def enabled? percent_enabled >

    0 && rand(100) < percent_enabled end def publish(result) end end
  13. class MyExperiment < ActiveRecord::Base include Scientist::Experiment def enabled? percent_enabled >

    0 && rand(100) < percent_enabled end def publish(result) $statsd.timing "science.#{name}.control", result.control.duration $statsd.timing "science.#{name}.candidate", result.candidate.duration end end
  14. class MyExperiment < ActiveRecord::Base include Scientist::Experiment def enabled? percent_enabled >

    0 && rand(100) < percent_enabled end def publish(result) $statsd.timing "science.#{name}.control", result.control.duration $statsd.timing "science.#{name}.candidate", result.candidate.duration if result.matched? $statsd.increment "science.#{name}.matched" elsif result.ignored? $statsd.increment "science.#{name}.ignored" else $statsd.increment "science.#{name}.mismatched" store_mismatch_data_in_redis(result) end end end
  15. # replace `Scientist::Default` as the implementation module Scientist::Experiment def self.new(name)

    MyExperiment.find_or_initialize_by(name: name) end end science "monster" do |experiment| experiment.class # => MyExperiment end
  16. class MyExperiment < ActiveRecord::Base include Scientist::Experiment def enabled? percent_enabled >

    0 && rand(100) < percent_enabled end def publish(result) $statsd.timing "science.#{name}.control", result.control.duration $statsd.timing "science.#{name}.candidate", result.candidate.duration if result.matched? $statsd.increment "science.#{name}.matched" elsif result.ignored? $statsd.increment "science.#{name}.ignored" else $statsd.increment "science.#{name}.mismatched" store_mismatch_data_in_redis(result) end end end
  17. class MyExperiment def enabled? return true if Rails.env.test? percent_enabled >

    0 && rand(100) < percent_enabled end end if Rails.env.test? MyExperiment.raise_on_mismatches = true end
  18. class Repository def pullable_by?(user) science "pullable_by" do |experiment| experiment.use {

    true } experiment.try { false } end end end test "repository is pullable by a collaborator" do assert repository.pullable_by?(collaborator) # => raises Scientist::Experiment::MismatchError end
  19. class Repository < ActiveRecord::Base include Scientist def pullable_by?(user) science "repository.pullable_by"

    do |e| e.use { pullable_by_legacy?(user) } e.try { pullable_by_abilities?(user) } end end end
  20. class Repository < ActiveRecord::Base include Scientist def pullable_by?(user) science "repository.pullable_by"

    do |e| e.context :user => user.id, :repository => id e.use { pullable_by_legacy?(user) } e.try { pullable_by_abilities?(user) } end end end
  21. class Repository < ActiveRecord::Base include Scientist def pullable_by?(user) science "repository.pullable_by"

    do |e| e.context :user => user.id, :repository => id e.use { pullable_by_legacy?(user) } e.try { pullable_by_abilities?(user) } e.ignore { user.staff? } end end end
  22. class Repository < ActiveRecord::Base include Scientist def pullable_by?(user) science "repository.pullable_by"

    do |e| e.context :user => user.id, :repository => id e.use { pullable_by_legacy?(user) } e.try { pullable_by_abilities?(user) } e.ignore { user.staff? } e.ignore do |control, candidate| # unconfirmed users aren't handled yet: control && !candidate && user.unconfirmed? end end end end
  23. class User def teams science "user.teams" do |e| e.use {

    legacy_teams } # DatabaseTeams e.try { new_teams } # WebServiceTeams end end end
  24. class User def teams science "user.teams" do |e| e.use {

    legacy_teams } # DatabaseTeams e.try { new_teams } # WebServiceTeams e.compare do |control, candidate| control == candidate end end end end
  25. class User def teams science "user.teams" do |e| e.use {

    legacy_teams } # DatabaseTeams e.try { new_teams } # WebServiceTeams e.compare do |control, candidate| control.map(&:id) == candidate.map(&:id) end end end end
  26. class Team def add_member(user) legacy_members << user # old system

    grant user, :read # new system end def grant(actor, permission) return unless Abilities.enabled? # ... end end
  27. class Team has_many :memberships has_many :users, :through => :memberships end

    class Membership belongs_to :team belongs_to :user end class User has_many :memberships has_many :teams, :through => :memberships end
  28. # Team <--> Membership <--> User Membership.create! team.users << user

    user.teams << team membership.destroy team.users.delete user user.teams.delete team
  29. User Repository Team read One team, with 10,000 users and

    5,000 repositories: 10,000 user → team records
  30. User Repository Team read write One team, with 10,000 users

    and 5,000 repositories: 10,000 user → team records 5,000 team → repository records
  31. User Repository Team read write write One team, with 10,000

    users and 5,000 repositories: 10,000 user → team records 5,000 team → repository records 50,000,000 user → repository records
  32. User Repository Team read write write One team, with 10,000

    users and 5,000 repositories: 10,000 user → team records 5,000 team → repository records 50,000,000 user → repository records
  33. class User def search_repositories(query) science "new-search-cluster" do e.use { search_old_cluster(query)

    } e.try { search_new_cluster(query) } e.compare { true } # don't care end end end
  34. ?