9 ms·

Move Fast and Fix Things

538 points by samlambert 11y ago

abritishguy 11y ago
Wow, strange that people weren't reporting these merge issues when they were clearly impacting people.
- slimsag 11y ago
  I haven't read the whole article yet, so I might have missed something; but how do we know that people weren't reporting these issues? I've always had to report issues to GitHub via email as they do not have a public issue tracker (something I've always found a bit ironic).
  - jessaustin 11y ago
    It's interesting that git is the same. [EDIT:] ...in that all issues and PRs are emailed rather than entered into a web app.
    - prtkgpt 11y ago
      git remains the same, indeed :)
    - slimsag 11y ago
      I'm not familiar with the Git project's inner workings, but their website git-scm.com tells me they are hosted on GitHub, which has a public issue tracker: https://github.com/git/git-scm.com/issues https://github.com/git/git-scm.com/issues GitHub, however, only allows issues to be reported via private mail. I'm not aware of a 'public' issue tracker for GitHub anywhere (even if a mailing list).
      - jessaustin 11y ago
        This is the web application for the git-scm.com site. It is meant to be the first place a person new to Git will land and download or learn about the Git SCM system. This app is written in Ruby on Rails and deployed on Heroku.
- nod 11y ago
  My read of the article implies that they were running the new method on the side, and comparing the results to the old method that was still running in production. They got to 100% before they actually pulled the lever on what customers would use. Edit: Ah, I see - talking about the Git bugs, not the differences . I’m actually not surprised that “256 (or a multiple) merge conflicts” was never noticed (or at least root-caused and fixed) by the entire git community. Wonderful ability to use a large userbase as a giant fuzzer.
  - pilif 11y ago
    I think OP was talking about the issue in git itself that caused merges with mod 256 conflicts to go though and be committed, including all the merge error markers. This happened on their live system (and would have happened on the command line for local git users), so OP (and incidentally, me too) was wondering how that wasn't noticed and wasn't causing support issues (it probably was which might have been another reason for this refactoring)
    - peff 11y ago
      I think it is a combination of two things: - the mod-256 conflict bug is exceedingly rare. Keep in mind that this is mod-256 individual hunk conflicts in a _single file_. Most files in a merge with conflicts have a handful of hunks. Over all of the testing at GitHub, only a single merge triggered this bug, and it was on a long repetitive file with automated changes. - the failure case was to quietly accept the merge. The result was obviously bogus, but didn't look any different than a user accidentally checking in the merge conflict markers. So if it was happening, I'd suspect that it went undiscovered either because the merge results were never used (e.g., it was a test-merge to feed the PR "Merge" button status) or the users simply scratched their head and fixed it.
  - mordocai 11y ago
    I would guess abritishguy was talking about the two git command line bugs they found.
smg 11y ago
I am trying to understand why the new merge method needed to be tested online via experiment. Both correctness and performance of the new merge method could have been tested offline working with snapshots (backups) of repos. Could a github engineer shed more light here?
- hueving 11y ago
  maybe they just do everything in production. "move fast and fix things"
- abritishguy 11y ago
  That is harder than just running the experiment online.
- kevan 11y ago
  Speculation, but if they already have the infrastructure to run the test online then it was probably easier than building one-time-use tools to test backups.
- tanoku 11y ago
  Author here. 5 years ago I would have agreed with you and logged e.g. 10 million merge requests to replay them offline. But one thing I've found over the years (which may seem obvious in retrospect) is that staging environment are not identical to production. Particularly not when it comes to finding sneaky bugs and performance regressions -- the code doesn't run on the same exact environment it will run when it is deployed (it has different input behaviors, and most importantly, it has different load and performance characteristics). The question then becomes "why would you run these experiments offline when you can run them online?". So we simply do. I personally feel it's a game changer.
  - solutionyogi 11y ago
    Couldn't agree more. It is almost impossible to have a staging environment which is EXACTLY like the Production environment. A slightest difference can introduce bug in Production which will get missed out in staging. And if you can run your experiment in Production without any downside, why not?
nod 11y ago
This is inspiring reading. One may not actually need the ability to deploy 60 times a day in order to refactor and experiment this effectively, but it's clearly a culture that will keep velocity high for the long-term.
- jeffjose 11y ago
  In the pursuit to get-things-done, we forget fundamentals and more often than not its the sound fundamentals that come in handy when your product has grown beyond your 1 or 2 member original tech team.
daveguy 11y ago
Very cool. I like this parallel execution of the original version and the update with comparisons between the two. They use a ruby package developed in house that has been made open source, Scientist. Does anyone know if there is an similar type package for python (preferably 2.7) development? It seems like an interesting area in between unit tests and A/B tests.
__jal 11y ago
Nothing really to contribute or ask, other than to say that I really enjoyed the writeup. Although I have nothing coming up that would use the code, the new library sounds really neat. Kudos!
mwcampbell 11y ago
This is tangential, but given the increasing functionality and maturity of libgit2, I wonder if it would yet be feasible to replace the Git command-line program with a new one based on libgit2, and written to be as portable as libgit2. Then there would be just one Git implementation, across the command line, GUIs, and web-based services like GitHub. Also, the new CLI could run natively on Windows, without MSYS.
- solutionyogi 11y ago
  That is my dream. Right now, many Windows GUIs (SmartGit/SourceTree) use 'git.exe' to manage the actual Git repositories. If libgit2 is fully mature, I can imagine more GUIs/tools will be built to manage/analyze the git repositories.
  - leonardinius 11y ago
    I believe github windows client is going that route https://github.com/blog/1127-github-for-windows https://github.com/blog/1127-github-for-windows. I also remember some blog post going into sync/async details of git.exe vs libgit2 stuff. Will try to google it.
    - MarkSweep 11y ago
      This is the article you speak of: http://githubengineering.com/git-concurrency-in-github-desktop/ http://githubengineering.com/git-concurrency-in-github-deskt... The AsyncReaderWriterLock mentioned in blog does not directly show up in Google, but it appears to be based on the one in this blog post: http://blogs.msdn.com/b/pfxteam/archive/2012/02/12/building-async-coordination-primitives-part-7-asyncreaderwriterlock.aspx http://blogs.msdn.com/b/pfxteam/archive/2012/02/12/building-...
  - dan15 11y ago
    > Right now, many Windows GUIs (SmartGit/SourceTree) use 'git.exe' to manage the actual Git repositories. For what it's worth, all of the Git functionality in Visual Studio uses libgit2.
- 11y ago
clebio 11y ago
Seems like the biggest takeaway is "have good tooling and instrumentation". I'm working with a complicated legacy production system, trying to rebuild pieces of it, and we have little or no instrumentation. Even _introducing_ such tooling is a potentially breaking change to production systems. Ach schade.
blt 11y ago
Github sounds like a great place to work.
jerf 11y ago
I'll highlight something I've learned in both succeeding and failing at this metric: When rewriting something, you should generally strive for a drop-in replacement that does the same thing, in some cases, even matching bug-for-bug, or, as in the article, taking a very close look at the new vs. the old bugs. It's tempting to throw away the old thing and write a brand new bright shiny thing with a new API and a new data models and generally NEW ALL THE THINGS!, but that is a high-risk approach that is usually without correspondingly high payoffs. The closer you can get to drop-in replacement, the happier you will be. You can then separate the risks of deployment vs. the new shiny features/bug fixes you want to deploy, and since risks tend to multiply rather than add, anything you can do to cut risks into two halves is still almost always a big win even if the "total risk" is still in some sense the same. Took me a lot of years to learn this. (Currently paying for the fact that I just sorta failed to do a correct drop-in replacement because I was drop-in replacing a system with no test coverage, official semantics, or even necessarily agreement by all consumers what it was and how it works, let alone how it should work.)
- humanrebar 11y ago
  > ...that is a high-risk approach that is usually without correspondingly high payoffs That's probably true, but it's also true that over a long enough timescale (100 years, to trigger a reductio ad absurdum) there is a very high risk that not replacing or rewriting that code will sink your technology and possibly your organization. Just because the risk will be realized in the long run doesn't mean it's not a risk. And if the worst-case scenario is death of the entire organization, then the math could very well add up to a full rewrite. Most business managers are not prepared to think strategically about long-term technical debt. It's the duty of engineers to let them know the difference between "not now" and "never". And the difference between "urgent" and "low priority".
  - mentat 11y ago
    It boils down to "don't change the implementation and the interface at the same time". It doesn't say "don't ever change the interface".
  - 11y ago
deleted 11y ago
[deleted]
- yarrel 11y ago
  The word "debt" is not just a financial term. There are debts of gratitude, debts to society, debts of honour, and so there are also technical debts. Objecting to the name "technical debt" on the basis that it is not the correct financial use of the term is like objecting to the name "work day" on the basis that it isn't measured in joules. It's a category error.
  - deleted 11y ago
    [deleted]
    - mixmastamyk 11y ago
      Yet you have not given one example of a better term. If you're not part of the solution, you're part of the precipitate... here at the bottom of the page.
      - deleted 11y ago
        [deleted]
dlib 11y ago
Very interesting, definitely gonna try this out as I have seen similar use-cases. Any change Github is at anytime going to show the specific merge-conflicts for a PR that cannot be merged?
eric_h 11y ago
> Finally, we removed the old implementation — which frankly is the most gratifying part of this whole process. On average, I get much more satisfaction from removing code than I do from adding new code. Admittedly, on occasion I'm very satisfied with new code, but on average, it's the removing that wins my heart.
- LaurentVB 11y ago
  I've long been dreaming of adding the following tagline to my resume: "Fixing bugs by removing code since 2006"
cantlin 11y ago
The strategy of proxying real usage to a second code path is incredibly effective. For months before the relaunch of theguardian.com, we ran traffic to the old site against the new stack to understand how it could be expected to perform in the real world. Later of course we moved real users, as incrementally as we possibly could. The hardest risk to mitigate is that users just won't like your new thing. But taking bugs and performance bottlenecks out of the picture ahead of time certainly ups your chances.
- geoka9 11y ago
  > The hardest risk to mitigate is that users just won't like your new thing. Do they ever? Why change the part users are used to?
- bertr4nd 11y ago
  Out of curiosity - when you've done this type of proxy test, what do you do about write operations? Do you proxy to a test DB, or do you have your code neatly factored to avoid writing on the test path (I guess most code I've worked on that needed a rewrite also wasn't neatly factored :) ).
danielsamuels 11y ago
I wish they would add the ability to fast-forward merge from pull requests. I know many large projects (including Django) accept pull requests but don't merge them on Github simply because of the mess it makes of the history.
- lumpypua 11y ago
  In my team's projects, code review in PRs made a mess of the history (certain devs in particular :P). We switched to a squash merge based workflow to address it, git reflow is our particular poison: https://github.com/reenhanced/gitreflow https://github.com/reenhanced/gitreflow
netghost 11y ago
For operations that don't have any side effects, I can definitely see how you could use the Science library. I'm curious though if there are any strategies folks use for experiments that do have side effects like updating a database or modifying files on disk.
- Nacraile 11y ago
  The first thing to do is to try to minimize the scope of mutating operations: e.g, decompose a monolithic read-write operation into independent load, transform, and store. You can then easily test as much as possible (the load and the store) side-by-side. The parts that must have side effects are still hard, but at least there are fewer of them. (One variant of this would be to build your code such that you can always intercept side-effects, and then block the new code's side effects and compare with the old code) The next thing is to try to take advantage of idempotence (and, by extension, try to make as many of your mutating operations idempotent as possible): there's nothing completely risk free, but you can at least verify idempotence for either ordering of new and old code, and if you're factored right, you can run both paths on the same input and verify they have the same side-effects and output. Finally, making the observation that the new code must in general be backwards-compatible with the old code, and both versions need to be able to run concurrently (because this situation will always exist during deployment): in the worst case, you can always start with a limited deployment of the new path, which limits the amount of damage done if the new code is bad.
- bcbrown 11y ago
  Point the control to the real database, and the experiment to an unused database. After each request through Scientist, the two databases should be identical. Or, make the output be the SQL command used to mutate the database. If the two outputs are different, then you've found something that needs investigation.
_yosefk 11y ago
TIL that github used to merge files differently than git because it used its own merge implementation based on git's code, to make it work on bare repos. Showcases a benefit of open formats and open source, showcases a downside as well (I'd never guess it might merge differently.) It's a good thing nobody contributes to my github repos since noone had the chance to run into the issue...
jcchee88 11y ago
When running with Scientist enabled, doesn't that mean you will add both the runtime of the old/new implementation instead of just one implementation? I could see this begin ok in most cases where speed is not a concern, but I wonder what we can do if we do care about speed?
- e28eta 11y ago
  We could go look at the implementation/documentation of Scientist to confirm, but nothing says the comparison has to be happening on the response thread. You could fork a new thread to run the unused implementation without impacting response time, as long as you have the server capacity.
- doppel 11y ago
  The article mentions running both code paths in parallel, so it becomes a matter of whether they have enough capacity for an extra cost for a (rather specific) path of their code. For this specifically I would imagine the cost is negligible in the big picture, considering regular activity far outnumbers pull/merge requests.
rcthompson 11y ago
How does Scientist work with code that produces side effects? In the example, presumably both the new and old each create a merge commit. Maybe these two merge commits are done in in-memory copies of the repo so that the test result can just be discarded, but what about in the general case where a function produces an output file or some other external effect?
- jdmichal 11y ago
  I would think that the operation would either (a) have to be pure or (b) be executed in two different environments. I think going for (a) is the easier approach. If you produce an output file, make a pure operation that generates the contents, then write it as a subsequent operation. Now you can test the contents against each other, but only actually write one of them. Basically, create an intermediary object that represents your state change and test those. Then "commit" the change from control and discard the one from the experiment.
- jsprogrammer 11y ago
  You'd only want to use pure functions in this manner. If external state is being modified, you can use a monad, or similar, to contain it.
cmrx64 11y ago
Does anyone know what an "O(n) issue" is? I can think of a few possible meanings in the usage here, but I've never heard it before and they all seem wrong.
- acveilleux 11y ago
  Using a linked-list where the actual access pattern is random and an hash table is more suitable would be the most obvious, especially since this is C code. Similar things includes C-style string functions like strlen() and so on that require iterating over an unknown length array. Caching (or better avoiding!) this work can save a lot of computing time. Example from libgit2: https://github.com/libgit2/libgit2/commit/7132150ddf7a883c1f12a89c2518c1a07c0dc94c https://github.com/libgit2/libgit2/commit/7132150ddf7a883c1f... In fact C-style string is a rich source of O(n) performance issues. And git is full of strings like filenames.
  - cmrx64 11y ago
    To me, I would have called that an "O(n^2)" (or whatever the proper non-linear order is) issue. The issue isn't that the algorithm is O(n), but that it's not O(n) when it could be! The other interpretations by andrewaylett and daveguy seem to be more accurate (and also agrees with my own thinking). But I could also see it being used for "the asymptotic complexity was hiding a huge constant". Have you heard your interpretation used in the wild?
- andrewaylett 11y ago
  It's an issue where processing something took O(n) time, when an algorithm to process it asymptotically more quickly (say O(log n)) would be more appropriate. The old code probably works properly for the average case, but will really blow up on pathological cases. My best personal example was an optimisation pass that turned out to be exponential in the worst case. On average `n` was small, so no-one cared, until a pathological case came up where compile time ballooned to over half-an-hour. Working out that the operation could be done in a linear way only shaved a fraction of a second off the average case, but brought the half-hour case down to a few seconds.
- 11y ago
openfuture 11y ago
Humans will always reverberate around truths like this. The emphasis shift on breaking vs fixing looks like a good example of how fashion trends in tech create artificial struggles that help new people understand the "boundaries" of $things. Fashion's like a tool for teaching via discussion Edit: I'm just commenting on what I percieve as a fashionable title not the article.

538 points by samlambert 11y ago

abritishguy 11y ago

Wow, strange that people weren't reporting these merge issues when they were clearly impacting people.

slimsag 11y ago
I haven't read the whole article yet, so I might have missed something; but how do we know that people weren't reporting these issues? I've always had to report issues to GitHub via email as they do not have a public issue tracker (something I've always found a bit ironic).
- jessaustin 11y ago
  It's interesting that git is the same. [EDIT:] ...in that all issues and PRs are emailed rather than entered into a web app.
  - prtkgpt 11y ago
    git remains the same, indeed :)
  - slimsag 11y ago
    I'm not familiar with the Git project's inner workings, but their website git-scm.com tells me they are hosted on GitHub, which has a public issue tracker: https://github.com/git/git-scm.com/issues https://github.com/git/git-scm.com/issues GitHub, however, only allows issues to be reported via private mail. I'm not aware of a 'public' issue tracker for GitHub anywhere (even if a mailing list).
    - jessaustin 11y ago
      This is the web application for the git-scm.com site. It is meant to be the first place a person new to Git will land and download or learn about the Git SCM system. This app is written in Ruby on Rails and deployed on Heroku.
nod 11y ago
My read of the article implies that they were running the new method on the side, and comparing the results to the old method that was still running in production. They got to 100% before they actually pulled the lever on what customers would use. Edit: Ah, I see - talking about the Git bugs, not the differences . I’m actually not surprised that “256 (or a multiple) merge conflicts” was never noticed (or at least root-caused and fixed) by the entire git community. Wonderful ability to use a large userbase as a giant fuzzer.
- pilif 11y ago
  I think OP was talking about the issue in git itself that caused merges with mod 256 conflicts to go though and be committed, including all the merge error markers. This happened on their live system (and would have happened on the command line for local git users), so OP (and incidentally, me too) was wondering how that wasn't noticed and wasn't causing support issues (it probably was which might have been another reason for this refactoring)
  - peff 11y ago
    I think it is a combination of two things: - the mod-256 conflict bug is exceedingly rare. Keep in mind that this is mod-256 individual hunk conflicts in a _single file_. Most files in a merge with conflicts have a handful of hunks. Over all of the testing at GitHub, only a single merge triggered this bug, and it was on a long repetitive file with automated changes. - the failure case was to quietly accept the merge. The result was obviously bogus, but didn't look any different than a user accidentally checking in the merge conflict markers. So if it was happening, I'd suspect that it went undiscovered either because the merge results were never used (e.g., it was a test-merge to feed the PR "Merge" button status) or the users simply scratched their head and fixed it.
- mordocai 11y ago
  I would guess abritishguy was talking about the two git command line bugs they found.

smg 11y ago

I am trying to understand why the new merge method needed to be tested online via experiment. Both correctness and performance of the new merge method could have been tested offline working with snapshots (backups) of repos. Could a github engineer shed more light here?

hueving 11y ago
maybe they just do everything in production. "move fast and fix things"
abritishguy 11y ago
That is harder than just running the experiment online.
kevan 11y ago
Speculation, but if they already have the infrastructure to run the test online then it was probably easier than building one-time-use tools to test backups.
tanoku 11y ago
Author here. 5 years ago I would have agreed with you and logged e.g. 10 million merge requests to replay them offline. But one thing I've found over the years (which may seem obvious in retrospect) is that staging environment are not identical to production. Particularly not when it comes to finding sneaky bugs and performance regressions -- the code doesn't run on the same exact environment it will run when it is deployed (it has different input behaviors, and most importantly, it has different load and performance characteristics). The question then becomes "why would you run these experiments offline when you can run them online?". So we simply do. I personally feel it's a game changer.
- solutionyogi 11y ago
  Couldn't agree more. It is almost impossible to have a staging environment which is EXACTLY like the Production environment. A slightest difference can introduce bug in Production which will get missed out in staging. And if you can run your experiment in Production without any downside, why not?

nod 11y ago

This is inspiring reading. One may not actually need the ability to deploy 60 times a day in order to refactor and experiment this effectively, but it's clearly a culture that will keep velocity high for the long-term.

jeffjose 11y ago
In the pursuit to get-things-done, we forget fundamentals and more often than not its the sound fundamentals that come in handy when your product has grown beyond your 1 or 2 member original tech team.

daveguy 11y ago

Very cool. I like this parallel execution of the original version and the update with comparisons between the two. They use a ruby package developed in house that has been made open source, Scientist. Does anyone know if there is an similar type package for python (preferably 2.7) development? It seems like an interesting area in between unit tests and A/B tests.

__jal 11y ago

Nothing really to contribute or ask, other than to say that I really enjoyed the writeup. Although I have nothing coming up that would use the code, the new library sounds really neat. Kudos!

mwcampbell 11y ago

This is tangential, but given the increasing functionality and maturity of libgit2, I wonder if it would yet be feasible to replace the Git command-line program with a new one based on libgit2, and written to be as portable as libgit2. Then there would be just one Git implementation, across the command line, GUIs, and web-based services like GitHub. Also, the new CLI could run natively on Windows, without MSYS.

solutionyogi 11y ago
That is my dream. Right now, many Windows GUIs (SmartGit/SourceTree) use 'git.exe' to manage the actual Git repositories. If libgit2 is fully mature, I can imagine more GUIs/tools will be built to manage/analyze the git repositories.
- leonardinius 11y ago
  I believe github windows client is going that route https://github.com/blog/1127-github-for-windows https://github.com/blog/1127-github-for-windows. I also remember some blog post going into sync/async details of git.exe vs libgit2 stuff. Will try to google it.
  - MarkSweep 11y ago
    This is the article you speak of: http://githubengineering.com/git-concurrency-in-github-desktop/ http://githubengineering.com/git-concurrency-in-github-deskt... The AsyncReaderWriterLock mentioned in blog does not directly show up in Google, but it appears to be based on the one in this blog post: http://blogs.msdn.com/b/pfxteam/archive/2012/02/12/building-async-coordination-primitives-part-7-asyncreaderwriterlock.aspx http://blogs.msdn.com/b/pfxteam/archive/2012/02/12/building-...
- dan15 11y ago
  > Right now, many Windows GUIs (SmartGit/SourceTree) use 'git.exe' to manage the actual Git repositories. For what it's worth, all of the Git functionality in Visual Studio uses libgit2.
11y ago

clebio 11y ago

Seems like the biggest takeaway is "have good tooling and instrumentation". I'm working with a complicated legacy production system, trying to rebuild pieces of it, and we have little or no instrumentation. Even _introducing_ such tooling is a potentially breaking change to production systems. Ach schade.

blt 11y ago

Github sounds like a great place to work.

jerf 11y ago

I'll highlight something I've learned in both succeeding and failing at this metric: When rewriting something, you should generally strive for a drop-in replacement that does the same thing, in some cases, even matching bug-for-bug, or, as in the article, taking a very close look at the new vs. the old bugs. It's tempting to throw away the old thing and write a brand new bright shiny thing with a new API and a new data models and generally NEW ALL THE THINGS!, but that is a high-risk approach that is usually without correspondingly high payoffs. The closer you can get to drop-in replacement, the happier you will be. You can then separate the risks of deployment vs. the new shiny features/bug fixes you want to deploy, and since risks tend to multiply rather than add, anything you can do to cut risks into two halves is still almost always a big win even if the "total risk" is still in some sense the same. Took me a lot of years to learn this. (Currently paying for the fact that I just sorta failed to do a correct drop-in replacement because I was drop-in replacing a system with no test coverage, official semantics, or even necessarily agreement by all consumers what it was and how it works, let alone how it should work.)

humanrebar 11y ago
> ...that is a high-risk approach that is usually without correspondingly high payoffs That's probably true, but it's also true that over a long enough timescale (100 years, to trigger a reductio ad absurdum) there is a very high risk that not replacing or rewriting that code will sink your technology and possibly your organization. Just because the risk will be realized in the long run doesn't mean it's not a risk. And if the worst-case scenario is death of the entire organization, then the math could very well add up to a full rewrite. Most business managers are not prepared to think strategically about long-term technical debt. It's the duty of engineers to let them know the difference between "not now" and "never". And the difference between "urgent" and "low priority".
- mentat 11y ago
  It boils down to "don't change the implementation and the interface at the same time". It doesn't say "don't ever change the interface".
- 11y ago

deleted 11y ago

[deleted]

yarrel 11y ago
The word "debt" is not just a financial term. There are debts of gratitude, debts to society, debts of honour, and so there are also technical debts. Objecting to the name "technical debt" on the basis that it is not the correct financial use of the term is like objecting to the name "work day" on the basis that it isn't measured in joules. It's a category error.
- deleted 11y ago
  [deleted]
  - mixmastamyk 11y ago
    Yet you have not given one example of a better term. If you're not part of the solution, you're part of the precipitate... here at the bottom of the page.
    - deleted 11y ago
      [deleted]

dlib 11y ago

Very interesting, definitely gonna try this out as I have seen similar use-cases. Any change Github is at anytime going to show the specific merge-conflicts for a PR that cannot be merged?

eric_h 11y ago

> Finally, we removed the old implementation — which frankly is the most gratifying part of this whole process. On average, I get much more satisfaction from removing code than I do from adding new code. Admittedly, on occasion I'm very satisfied with new code, but on average, it's the removing that wins my heart.

LaurentVB 11y ago
I've long been dreaming of adding the following tagline to my resume: "Fixing bugs by removing code since 2006"

cantlin 11y ago

The strategy of proxying real usage to a second code path is incredibly effective. For months before the relaunch of theguardian.com, we ran traffic to the old site against the new stack to understand how it could be expected to perform in the real world. Later of course we moved real users, as incrementally as we possibly could. The hardest risk to mitigate is that users just won't like your new thing. But taking bugs and performance bottlenecks out of the picture ahead of time certainly ups your chances.

geoka9 11y ago
> The hardest risk to mitigate is that users just won't like your new thing. Do they ever? Why change the part users are used to?
bertr4nd 11y ago
Out of curiosity - when you've done this type of proxy test, what do you do about write operations? Do you proxy to a test DB, or do you have your code neatly factored to avoid writing on the test path (I guess most code I've worked on that needed a rewrite also wasn't neatly factored :) ).

danielsamuels 11y ago

I wish they would add the ability to fast-forward merge from pull requests. I know many large projects (including Django) accept pull requests but don't merge them on Github simply because of the mess it makes of the history.

lumpypua 11y ago
In my team's projects, code review in PRs made a mess of the history (certain devs in particular :P). We switched to a squash merge based workflow to address it, git reflow is our particular poison: https://github.com/reenhanced/gitreflow https://github.com/reenhanced/gitreflow

netghost 11y ago

For operations that don't have any side effects, I can definitely see how you could use the Science library. I'm curious though if there are any strategies folks use for experiments that do have side effects like updating a database or modifying files on disk.

Nacraile 11y ago
The first thing to do is to try to minimize the scope of mutating operations: e.g, decompose a monolithic read-write operation into independent load, transform, and store. You can then easily test as much as possible (the load and the store) side-by-side. The parts that must have side effects are still hard, but at least there are fewer of them. (One variant of this would be to build your code such that you can always intercept side-effects, and then block the new code's side effects and compare with the old code) The next thing is to try to take advantage of idempotence (and, by extension, try to make as many of your mutating operations idempotent as possible): there's nothing completely risk free, but you can at least verify idempotence for either ordering of new and old code, and if you're factored right, you can run both paths on the same input and verify they have the same side-effects and output. Finally, making the observation that the new code must in general be backwards-compatible with the old code, and both versions need to be able to run concurrently (because this situation will always exist during deployment): in the worst case, you can always start with a limited deployment of the new path, which limits the amount of damage done if the new code is bad.
bcbrown 11y ago
Point the control to the real database, and the experiment to an unused database. After each request through Scientist, the two databases should be identical. Or, make the output be the SQL command used to mutate the database. If the two outputs are different, then you've found something that needs investigation.

_yosefk 11y ago

TIL that github used to merge files differently than git because it used its own merge implementation based on git's code, to make it work on bare repos. Showcases a benefit of open formats and open source, showcases a downside as well (I'd never guess it might merge differently.) It's a good thing nobody contributes to my github repos since noone had the chance to run into the issue...

jcchee88 11y ago

When running with Scientist enabled, doesn't that mean you will add both the runtime of the old/new implementation instead of just one implementation? I could see this begin ok in most cases where speed is not a concern, but I wonder what we can do if we do care about speed?

e28eta 11y ago
We could go look at the implementation/documentation of Scientist to confirm, but nothing says the comparison has to be happening on the response thread. You could fork a new thread to run the unused implementation without impacting response time, as long as you have the server capacity.
doppel 11y ago
The article mentions running both code paths in parallel, so it becomes a matter of whether they have enough capacity for an extra cost for a (rather specific) path of their code. For this specifically I would imagine the cost is negligible in the big picture, considering regular activity far outnumbers pull/merge requests.

rcthompson 11y ago

How does Scientist work with code that produces side effects? In the example, presumably both the new and old each create a merge commit. Maybe these two merge commits are done in in-memory copies of the repo so that the test result can just be discarded, but what about in the general case where a function produces an output file or some other external effect?

jdmichal 11y ago
I would think that the operation would either (a) have to be pure or (b) be executed in two different environments. I think going for (a) is the easier approach. If you produce an output file, make a pure operation that generates the contents, then write it as a subsequent operation. Now you can test the contents against each other, but only actually write one of them. Basically, create an intermediary object that represents your state change and test those. Then "commit" the change from control and discard the one from the experiment.
jsprogrammer 11y ago
You'd only want to use pure functions in this manner. If external state is being modified, you can use a monad, or similar, to contain it.

cmrx64 11y ago

Does anyone know what an "O(n) issue" is? I can think of a few possible meanings in the usage here, but I've never heard it before and they all seem wrong.

acveilleux 11y ago
Using a linked-list where the actual access pattern is random and an hash table is more suitable would be the most obvious, especially since this is C code. Similar things includes C-style string functions like strlen() and so on that require iterating over an unknown length array. Caching (or better avoiding!) this work can save a lot of computing time. Example from libgit2: https://github.com/libgit2/libgit2/commit/7132150ddf7a883c1f12a89c2518c1a07c0dc94c https://github.com/libgit2/libgit2/commit/7132150ddf7a883c1f... In fact C-style string is a rich source of O(n) performance issues. And git is full of strings like filenames.
- cmrx64 11y ago
  To me, I would have called that an "O(n^2)" (or whatever the proper non-linear order is) issue. The issue isn't that the algorithm is O(n), but that it's not O(n) when it could be! The other interpretations by andrewaylett and daveguy seem to be more accurate (and also agrees with my own thinking). But I could also see it being used for "the asymptotic complexity was hiding a huge constant". Have you heard your interpretation used in the wild?
andrewaylett 11y ago
It's an issue where processing something took O(n) time, when an algorithm to process it asymptotically more quickly (say O(log n)) would be more appropriate. The old code probably works properly for the average case, but will really blow up on pathological cases. My best personal example was an optimisation pass that turned out to be exponential in the worst case. On average `n` was small, so no-one cared, until a pathological case came up where compile time ballooned to over half-an-hour. Working out that the operation could be done in a linear way only shaved a fraction of a second off the average case, but brought the half-hour case down to a few seconds.
11y ago

openfuture 11y ago

Humans will always reverberate around truths like this. The emphasis shift on breaking vs fixing looks like a good example of how fashion trends in tech create artificial struggles that help new people understand the "boundaries" of $things. Fashion's like a tool for teaching via discussion Edit: I'm just commenting on what I percieve as a fashionable title not the article.