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.

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. ?