Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Td 4804 oom fix v2 #5464

Closed
wants to merge 16 commits into from
Closed

Commits on Jul 5, 2022

  1. Clarify behavior of process_next_response

    The behavior of process_next_response is worth clarifying as the
    returned future does not nececessarily wait for all enqueued respones
    to be finished before resolving.
    
    We also rename the method to better reflect its purpose.
    travisdowns committed Jul 5, 2022
    Configuration menu
    Copy the full SHA
    8285f4f View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    f74e577 View commit details
    Browse the repository at this point in the history
  3. Release resources after the response is written

    Currently we release resources after the response is enqueued
    in connection_context and response processing is called, but it
    may not have been sent at this point as we require in-order responses
    but second-stage processing may happen out of order.
    
    In this change, we instead tunnel the resource object through to
    the place where the response is written, and release it there.
    
    FIxes redpanda-data#5278.
    travisdowns committed Jul 5, 2022
    Configuration menu
    Copy the full SHA
    0177bb4 View commit details
    Browse the repository at this point in the history
  4. Configuration menu
    Copy the full SHA
    1b07f98 View commit details
    Browse the repository at this point in the history
  5. Move max_api_key function to handlers header.

    This lets us share it with the request processing code which
    would also like to do type list based manipulation of the
    request types.
    travisdowns committed Jul 5, 2022
    Configuration menu
    Copy the full SHA
    66ebbe8 View commit details
    Browse the repository at this point in the history
  6. Introduce KafkaApiHandlerAny concept

    We already had concepts for one-phase and two-phase handlers,
    and this concept is simply the union of those two handler
    concepts, i.e., "any" type of handler.
    travisdowns committed Jul 5, 2022
    Configuration menu
    Copy the full SHA
    9340d4a View commit details
    Browse the repository at this point in the history
  7. Introduce type-erased any_handler

    This is a polymorphic handler each of which is backed by an existing
    concrete handler, but which lets us treat handlers generically without
    restorting to template functions.
    
    This reduces code bloat significantly as we do not duplicate code paths
    for our ~45 handler types.
    
    For example, requests.cc.o drops from ~11 MB to ~5 MB after it is
    switched to the any_handler approach.
    travisdowns committed Jul 5, 2022
    Configuration menu
    Copy the full SHA
    40a9201 View commit details
    Browse the repository at this point in the history
  8. Add any_handler unit tests

    The any_handler already gets good functional coverage as it is
    added to the core request path in requests.cc, but we also include
    this unit test with basic coverage.
    travisdowns committed Jul 5, 2022
    Configuration menu
    Copy the full SHA
    2dc759c View commit details
    Browse the repository at this point in the history
  9. Update request handling to use polymorphic handler

    Preceding changes in this series introduced a runtime polymorphic
    handler class, and this change switches most of the request handling
    to use it.
    
    In particular, we replace the large switch on API key which dispatches
    to a template method to a lookup of the handler method and virtual
    dispatch.
    
    Some handlers that need special processing like the authentication/SASL
    related ones still use the old approach for now.
    travisdowns committed Jul 5, 2022
    Configuration menu
    Copy the full SHA
    f07cdc9 View commit details
    Browse the repository at this point in the history

Commits on Jul 6, 2022

  1. Add support for memory estimation to handlers

    Currently we use  single memory estimate for all kafka request types,
    but different API calls may use wildly different amounts of memory.
    
    This change allows each handler to perform an API-specific calculation
    instead.
    travisdowns committed Jul 6, 2022
    Configuration menu
    Copy the full SHA
    31256f9 View commit details
    Browse the repository at this point in the history
  2. Use handler specific memory estimate

    In connection_context, we now use the handler-specific initial memory
    use estimate, rather than a single estimate for every handler type.
    travisdowns committed Jul 6, 2022
    Configuration menu
    Copy the full SHA
    172a457 View commit details
    Browse the repository at this point in the history

Commits on Jul 10, 2022

  1. Move session_resouces object to top level

    The session_resources type was a private member of
    connection_context, but as we want to use it more
    broadly, move it out as a standalone public class.
    
    Additionally, pass it by shared_pointer in preparation
    for later changes will feed it into requests.
    travisdowns committed Jul 10, 2022
    Configuration menu
    Copy the full SHA
    ff824fc View commit details
    Browse the repository at this point in the history

Commits on Jul 11, 2022

  1. Use a conservative estimator for metadata requests

    Currently we estimate that metadata requests take 8000 + rsize * 2 bytes
    of memory to process, where rsize is the size of the request. Since
    metadata requests are very small, this end up being roughly 8000 bytes.
    
    However, metadata requests which return information about every
    partition and replica may easily be several MBs in size.
    
    To fix this for metadata requests specifically, we use a new more
    conservative estimate which uses the "maximum supported"
    partition count give an upper bound on the size. This this ends up
    "only" 8 MB, and applies only to metadata requests, the performance
    cost (chiefly a reduced maximum number of metadata requests in flight on
    one shard) should be moderate.
    travisdowns committed Jul 11, 2022
    Configuration menu
    Copy the full SHA
    dbd8134 View commit details
    Browse the repository at this point in the history

Commits on Jul 13, 2022

  1. Configuration menu
    Copy the full SHA
    0c0462a View commit details
    Browse the repository at this point in the history

Commits on Jul 14, 2022

  1. Introduce handler template for two-stage handlers

    Single-stage handlers have a hander template which means that handler
    objects can be declared in a single line specifying their api object,
    min and max API versions.
    
    This change extends this nice concept to two-stage handlers as well.
    travisdowns committed Jul 14, 2022
    Configuration menu
    Copy the full SHA
    4004d72 View commit details
    Browse the repository at this point in the history
  2. Add the connection context to the memory estimator

    Passing the connection context to the estimator allows the
    estimator to use the various subsystems to estimate the memory
    use of a given request.
    travisdowns committed Jul 14, 2022
    Configuration menu
    Copy the full SHA
    5429713 View commit details
    Browse the repository at this point in the history