Comparing develop...codesing-test · twitter/finagle · GitHub
Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: twitter/finagle
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: develop
Choose a base ref
...
head repository: twitter/finagle
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: codesing-test
Choose a head ref
Checking mergeability… Don’t worry, you can still create the pull request.
  • 3 commits
  • 14 files changed
  • 3 contributors

Commits on Jan 29, 2021

  1. finagle-zipkin-core: add annotation for sampling rate at trace roots 

    Problem:
We need a better understanding of how sampleRate works in production
in order to make sure the flag `initialSampleRate` is working.

Solution:
Change behavior of Sampling Tracer to add an annotation,
`zipkin.sampling_rate`, at trace roots
GitOrigin-RevId: c1ee6bf17f73eee5c4cdbbac6568706db862911a
    @joybestourous Twitter Service
    joybestourous authored and Twitter Service committed Jan 29, 2021
    Configuration menu
    Copy the full SHA
    4c06784 View commit details
    Browse the repository at this point in the history

  2. finagle-core: Better OffloadFilter position (under a toggle) 

    Problem

Today's OffloadFilter is sitting on the edge between the application and the Finagle stack, forking execution
just as it enters the userland. This approach implies a suboptimal near-CPU performance as most of the data
needed to handle a request won't be cache-ready.

Solution

Fork execution earlier on the edge between the Netty pipeline and the Finagle stack. This way all data required
by the application logic will be waiting in the cache. The new position is placed under a toggle (disabled by default)
for a smooth and gradual roll out.

Result

Application with offload filter enabled just became a little faster. Forking into the application thread earlier also
means there is less work to be done by the IO thread, which presumably means the Netty worker pool can be shrunk
even further (if needed).

GitOrigin-RevId: bf048ab0fb0c3882b73aacdb7c6129309d1660d2
    @vkostyukov Twitter Service
    vkostyukov authored and Twitter Service committed Jan 29, 2021
    Configuration menu
    Copy the full SHA
    839fa65 View commit details
    Browse the repository at this point in the history

  3. util-routing: evolve routing result beyond Option[Route] and simplify… 

    … Router generation

Problem

A `c.t.util.routing.Router` represents the result of finding
a `Route` as a `Option[Route]`. As we implement new
use cases around the `Router`, we have found a need
to pass along the routing `Input` as part of the result.

Solution

Introduce a new `c.t.util.routing.Result` type for representing
a `Router` result, which can be represented by different
states: `Found` and `NotFound`. As a side-effect, we have
the ability to return a result that represents a router
being closed instead of being forced to throw an exception
for states that do not match Some or None. This also
simplifies our Java-friendly story and allows us to remove
`AbstractRouter`.

It was also found that the `Generator` should use an
encapsulating type for its input instead of a Tuple, which
has also been reflected in these changes.

Result

The result of routing via a `Router` can pass on more
information than was previously possible. Generating a
`Router` is simplified.

GitOrigin-RevId: 2741ba90522c6f21f882e4c710256532e24822d4
    @enbnt Twitter Service
    enbnt authored and Twitter Service committed Jan 29, 2021
    Configuration menu
    Copy the full SHA
    a22b93b View commit details
    Browse the repository at this point in the history
Loading