Uploaded image for project: 'Couchbase nginx Module'
  1. Couchbase nginx Module
  2. NGXCBM-2

Feedback from initial module delivery

    XMLWordPrintable

Details

    • Bug
    • Resolution: Fixed
    • Major
    • 0.2
    • 0.1
    • core
    • None

    Description

      These are mutliple items from the customer's feedback. Let me know if you would prefer them split into separate bugs.

      -First issue is related to the fact, that during operation of nginx worker process all configuration records must remain constant. In addition to that, the lifetime of configuration records might be different than the lifetime of runtime structures. In the version being examined there are modifications of configuration records (e.g. ngx_http_couchbase_loc_conf_t.connected), as well as usage of data referenced from configuration records outside of their lifetime (ngx_http_couchbase_loc_conf_t.lcb and ngx_http_couchbase_main_conf_t.lcb_io). This issue could lead to a situation when unallocated memory is dereferenced by libcouchbase.

      -Recommended usage pattern is to allocate all configuration settings during configuration phase and all runtime structures in module's init_process handler.

      -In common dynamic memory allocators release operation is usually slower due to the fact, that it requires reconciliation of neighboring memory blocks to avoid fragmentation. Nginx memory allocator avoids this problem by allocating memory from pools. Each pool consists of large chunks 4-16 kilobytes long. Each pool starts with a single chunk of pre-defined size. Small objects are quickly allocated from chunks, while large objects are allocated directly from dynamic memory. Once a chunk is exhausted, Nginx allocates next chunk and uses it to allocated small objects.

      Each pool is usually associated with an object of known lifetime, such as connection or request. When such object ceases to exist, pool is released by releasing all chunks at once. In this way fragmentation is minimized as well as costs of allocating and freeing small objects.

      When a request is involved in continuous data transfer, it is usually sufficient to allocate a limited set of buffers per request and reuse them while transferring data. This avoids spikes in memory consumption that are undesirable for higly-loaded system.

      Below is a complete list of issues in order of significance and improvement recommendations.
      1: ngx_lcb_recvv, ngx_lcb_sendv - Allocations can lead to excessive memory consumption, Allocate buffers and chains once per request
      2: ngx_http_couchbase_handler - Body might be corrupted due to call to both ngx_http_discard_request_body and ngx_http_read_client_request_body, Decide whether to discard or read body before proceeding with further actions
      3: callbacks - ngx_http_send_header and ngx_http_output_filter might return not only NGX_AGAIN and this is fine, Expect other return codes, such as NGX_AGAIN from ngx_http_send_header and ngx_http_output_filter
      4: callbacks - Neither of callbacks invoke ngx_http_finalize_request, Make sure ngx_http_finalize_request for each request
      5: ngx_http_couchbase_process - Body might not be entirely loaded, Limit body length and instruct nginx to load entire body
      6: ngx_http_couchbase_process - No check for ngx_palloc return code, Check for ngx_palloc return code
      7: ngx_http_couchbase_upstrea m_init - A race condition might lead to connection leak, Follow nginx standard process flow
      8: ngx_http_couchbase_lcb_opti ons - Memory leak when doing goto invalid, Do not use memory allocation API from standard library
      9: ngx_lcb_connect_peer - Call to ngx_add_event after ngx_event_connect_peer is redundant, See internals of ngx_event_connect_peer : the write event is armed by default
      10: ngx_lcb_connect_peer - Connect timeout is not configurable Make connection timeout configurable

      Attachments

        No reviews matched the request. Check your Options in the drop-down menu of this sections header.

        Activity

          People

            avsej Sergey Avseyev
            perry Perry Krug
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Gerrit Reviews

                There are no open Gerrit changes

                PagerDuty