twitter-archive / diffy

Find potential bugs in your services with Diffy
https://twitter.com/diffyproject
Apache License 2.0
3.83k stars 368 forks source link

Diffy Return Response Of Primary #5

Closed cross311 closed 4 years ago

cross311 commented 9 years ago

It seems as though the response from the clients is not being returned, but always just returning empty response.

in DifferenceProxy.scala line 92 NoReponseException is returned always.

It would be nice to be able to run this under a dev website that is pointing to the proxy and have the results passed through.

Question I can see is what do you return the primary or the candidate.

puneetkhanduri commented 9 years ago

The use case you've proposed makes sense but let me share the motivation behind our current implementation.

We currently return an exception to respond as quickly as possible to the client without blocking on responses from any of the test instances (primary, secondary, candidate). Our premise is that some resource (thread/task/memory) is blocked on the client side (whoever is sending test requests to diffy) and should be freed as quickly as possible.

This is specially relevant when you eavesdrop on your production clusters to sample and emit large volumes of one-way "dark" traffic to Diffy. Whatever instrumentation sends this traffic to Diffy does not care about the response as it's only purpose is to send a sufficiently large volume of traffic while consuming the minimal amount of resources.

Ultimately, the constant exception is just about being able respond to any type of request without any blocking on the Diffy side.

cross311 commented 9 years ago

Just to make sure I understand: Given current implementation of Diffy, the only way to truly utilize its power is to add an additional piece of software/infrastructure to record and play traffic against the Diffy proxy. Given that Diffy does not return anything that is usable for upstream consumers.

Does the Diffy team plan on releasing a tool to help with this? That seems like a crucial part of the puzzle.

I can see other approaches:

I would really love to utilize Diffy, just having a hard time justifying the additional infrastructure needed to utilize.

cross311 commented 9 years ago

Can you expand on how Twitter uses Diffy? I realize you might not be able to because it is not authorized public knowledge. I would find it helpful.

Thanks for your response!

coderunner commented 9 years ago

What about passing a command line flag to determine if a response is to be returned or not?

Looking at the code, I saw that the current implementation calls all the backing services in sequence. I also saw some parallel implementation but that seems unused.

Question: Why sequential access has been chosen?

If passing a flag is acceptable, in order to send the response quickly, I think diffy should return the primaryService (or the flag could specify which response to return) response without waiting for anything else to be processed.

As a prototype, I modified slightly the DifferenceProxy#proxy method to query all services in parallel and return the future of the primary server response.

Does that make sense? Would you accept a PR that introduces a flag to return a response?

def proxy = new Service[Req, Rep] {
    override def apply(req: Req): Future[Rep] = {
      val rawResponses = Seq(primary.client, candidate.client, secondary.client) map { service =>
        service(req).liftToTry
      }

      val responses: Future[Seq[Message]] =
        Future.collect(rawResponses) flatMap { reps =>
          Future.collect(reps map liftResponse) respond {
            case Return(rs) =>
              log.debug(s"success lifting ${rs.head.endpoint}")

            case Throw(t) => log.debug(t, "error lifting")
          }
        }

      responses foreach {
        case Seq(primaryResponse, candidateResponse, secondaryResponse) =>
          liftRequest(req) respond {
            case Return(m) =>
              log.debug(s"success lifting request for ${m.endpoint}")

            case Throw(t) => log.debug(t, "error lifting request")
          } foreach { req =>
            analyzer(req, candidateResponse, primaryResponse, secondaryResponse)
          }
      }

      rawResponses.head.flatMap { Future.const }
    }
  }
puneetkhanduri commented 9 years ago

I was in the process of composing a response proposing the approach of adding a command line flag. A pull request is welcome.

Regarding parallel vs sequential. We actually have code that does that in the proxy package but we moved from parallel to sequential as it serves as a noise reduction trick when used in primary -> candidate -> secondary sequence. This helps when the underlying data may be live and skews the odds of noise in favor of candidate. i.e. Primary is now more likely to disagree Secondary than Candidate because the request to Secondary is more delayed than the request to Candidate (relative to Primary) and underlying data is more likely to change over a longer time interval than a shorter one.

derEremit commented 8 years ago

That was pretty confusing for me! As I always thought I should get a response from a proxy! Please at least mention that in the README

derEremit commented 8 years ago

btw: for duplicating traffic at least on a test setup: https://github.com/agnoster/duplicator