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.

Sign up for GitHub

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

Jump to bottom

colexecargs: fix recent memory leak #64453

Merged
merged 4 commits into from
May 5, 2021
Merged

colexecargs: fix recent memory leak #64453

merged 4 commits into from
May 5, 2021

Commits on May 5, 2021

  1. colfetcher: set all unneeded vectors to all nulls 

    The cFetcher creates a batch with the same schema as the table it is
reading from. In many cases not all columns from the table are needed
for the query, so as an important perfomance optimization the cFetcher
doesn't decode the data for those columns. As a result, such unneeded
columns are left "unset". This works ok in most cases, however, if we
attempt to materialize such a batch with unset vectors, the conversion
to datums might encounter errors (e.g. UUID values must either be NULL
or have 16 bytes length).

This commit improves this situation slightly by tracking the set of
unneeded columns and setting those vectors to all NULL values. This
will allow to simplify the planning code a bit in the follow-up commit.

Release note: None
    @yuzefovich
    yuzefovich committed May 5, 2021
    Configuration menu
    Copy the full SHA
    2223a60 View commit details
    Browse the repository at this point in the history

  2. colbuilder: remove unnecessary complication when wrapping table reader 

    Previously, we had some custom code for the case when we supported the
table reader core but not the post-processing spec - we attempted to
revert to the core-pre-planning state and plan the whole table reader
with render expressions on top.

Given the previous commit, I think this is no longer necessary, so this
commit removes that special code in favor of the general handling of
only the post-processing spec via a noop processor. This commit was
prompted by some complications because of this old code for the
follow-up commit.

Release note: None
    @yuzefovich
    yuzefovich committed May 5, 2021
    Configuration menu
    Copy the full SHA
    b65433e View commit details
    Browse the repository at this point in the history

  3. colexecargs: fix recent memory leak 

    In c3b1617 we introduced a new utility
struct that keeps information about the meta objects in the operator
tree. Those meta objects are tracked by several slices which are
resliced to be of length 0 when the "outer" object is released back to
the corresponding pool. However, the slices still end up holding
references to the old meta objects prohibiting those from being GCed.
Such a behavior results in a memory leak. This commit fixes the issue by
explicitly resetting the slices for reuse.

Release note: None
    @yuzefovich
    yuzefovich committed May 5, 2021
    Configuration menu
    Copy the full SHA
    31f5d28 View commit details
    Browse the repository at this point in the history

  4. sql: audit implementations of Releasable interface of slices' reuse 

    This commit performed the audit of all slices that are kept by
components implementing `execinfra.Releasable` interface to make sure
that the slices that might be referencing large objects are deeply
reset. (By deep reset I mean all slots are set to `nil` so that the
possibly large objects could be garbage-collected.) This was prompted by
the previous commit which fixed a recent regression, but this commit
seems like a good idea on its own, and it might be worth backporting it
too.

Release note: None
    @yuzefovich
    yuzefovich committed May 5, 2021
    Configuration menu
    Copy the full SHA
    99153b4 View commit details
    Browse the repository at this point in the history