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

Extremely high memory usage in 1.1.0-pre with moderately sized messages #9510

Closed
sztomi opened this Issue Jan 30, 2017 · 9 comments

Comments

Projects
None yet
4 participants
@sztomi

sztomi commented Jan 30, 2017

I'm creating a benchmark of RPC libraries where a client and server are created in the same process and the performance of message passing is measured in each iteration. Creating the server and client happens once per benchmark, i.e. not per iteration.

One of the benchmarks uses cached large blobs of increasing sizes from 4k up to 32M. Using grpc 1.1.0-pre with the protobuf it references as a submodule, I consistently get several gigabytes of memory usage once the benchmark reaches the 2048k size (not sure if that's not from the previous 1024k step). Unfortunately I can't tell how much, because it eats all the memory I have in this machine (8 GB).

I was able to reproduce this consistently on both my host machine (Arch x64, g++6.3) and a VM (Ubuntu 16.04 x64, g++5.4).

I reduced my benchmark to only contain grpc code, this is the gist of it:

class grpc_service : public GrpcServiceBenchmark::Service {
public:
  ::grpc::Status get_blob(::grpc::ServerContext *context,
                            const ::EmptyRequest *request,
                            ::BlobResponse *response) override {
      response->set_data(::get_blob(blob_size_));
      return ::grpc::Status::OK;
  }
  int blob_size_;
};

class grpc_bench : public benchmark::Fixture {
public:
  grpc_bench()
      : channel_(grpc::CreateChannel(server_addr_,
                                     grpc::InsecureChannelCredentials())),
        client_(channel_) {
    grpc::ServerBuilder b;
    b.AddListeningPort(server_addr_, grpc::InsecureServerCredentials());
    b.RegisterService(&service_impl_);
    server_ = b.BuildAndStart();
  }
  void get_blob(int param) {
    service_impl_.blob_size_ = param;
    grpc::ClientContext client_context;
    EmptyRequest request;
    BlobResponse response;
    auto status = client_.get_blob(&client_context, request, &response);
    std::string s;
    benchmark::DoNotOptimize(s = response.data());
    std::size_t size;
    benchmark::DoNotOptimize(size = s.size());
  }

  ~grpc_bench() noexcept { server_->Shutdown(); }

  std::shared_ptr<grpc::ChannelInterface> channel_;
  GrpcServiceBenchmark::Stub client_;
  std::unique_ptr<grpc::Server> server_;
  grpc_service service_impl_;
  static constexpr const char *server_addr_ = "localhost:8082";
};

The benchmark calls get_blob with increasing sizes through a large number of iterations.

You can clone this branch for the full source: https://github.com/rpclib/benchmarks/tree/grpc_high_memory_repro

To build the above repo, conan is required:

mkdir build && cd build
conan install .. --build missing
cmake ..
make

There is of course a chance that I made a mistake in my benchmarking code. Please let me know if I can provide any more details.

Also, I would be interested in you opinion about whether or not this code is a fair entry in this benchmark, as I'm not very knowledgeable about grpc. You can see the implementations using other libraries on the master branch of the same repo for comparison.

@ctiller

This comment has been minimized.

Show comment
Hide comment
@ctiller

ctiller Jan 30, 2017

Member
Member

ctiller commented Jan 30, 2017

@sztomi

This comment has been minimized.

Show comment
Hide comment
@sztomi

sztomi Jan 30, 2017

Yes (at least that was my intention, but I'm really new to grpc).

sztomi commented Jan 30, 2017

Yes (at least that was my intention, but I'm really new to grpc).

@sztomi

This comment has been minimized.

Show comment
Hide comment
@sztomi

sztomi Feb 4, 2017

This issue is still reproducible with the finalized release (https://github.com/grpc/grpc/releases/tag/v1.1.0). I have updated my conan package and repro branch to refer to this version (see above).

sztomi commented Feb 4, 2017

This issue is still reproducible with the finalized release (https://github.com/grpc/grpc/releases/tag/v1.1.0). I have updated my conan package and repro branch to refer to this version (see above).

@sztomi

This comment has been minimized.

Show comment
Hide comment
@sztomi

sztomi Feb 20, 2017

I was wondering if this is going to be fixed in the near future? I'd like to release a blog post about the benchmark suite I created, but I'd have to exclude grpc from this particular benchmark due to this bug (two other benchmarks can be performed with the current release though fortunately). Am I wrong in assuming that this potentially makes grpc vulnerable to resource exhaustion attacks?

sztomi commented Feb 20, 2017

I was wondering if this is going to be fixed in the near future? I'd like to release a blog post about the benchmark suite I created, but I'd have to exclude grpc from this particular benchmark due to this bug (two other benchmarks can be performed with the current release though fortunately). Am I wrong in assuming that this potentially makes grpc vulnerable to resource exhaustion attacks?

@ctiller

This comment has been minimized.

Show comment
Hide comment
@ctiller

ctiller Feb 21, 2017

Member

@lyuxuan - can you try and reproduce this and capture where the allocations are coming from?

Member

ctiller commented Feb 21, 2017

@lyuxuan - can you try and reproduce this and capture where the allocations are coming from?

@lyuxuan

This comment has been minimized.

Show comment
Hide comment
@lyuxuan

lyuxuan Mar 15, 2017

Contributor

@sztomi sorry about the late reply. I cloned your code on a VM and tried to reproduce it, but I have some issues with conan.
Running the "conan install .. --build missing", I got the error shown below:
ERROR: pkg-config/0.29.1@sztomi/testing: Error in source() method, line 21
tools.download(tarball_url, tgz)
Error downloading file https://pkgconfig.freedesktop.org/releases/pkg-config-0.29.1.tar.gz: 'hostname 'pkgconfig.freedesktop.org' doesn't match either of 'distributions.freedesktop.org', 'farsight.freedesktop.org', 'fontconfig.freedesktop.org', 'fontconfig.org', 'freedesktop.org', 'geoclue.freedesktop.org', 'secure.freedesktop.org', 'www.fontconfig.org', 'www.freedesktop.org''

and then I added verify=false to download(). New error appeared:
thrift/0.9.3@sztomi/testing: ERROR: Package '5803b7ddb711007bf721b44c0023e9272981760a' build failed
thrift/0.9.3@sztomi/testing: WARN: Build folder /home/yuxuanli/.conan/data/thrift/0.9.3/sztomi/testing/build/5803b7ddb711007bf721b44c0023e9272981760a
thrift/0.9.3@sztomi/testing: WARN: File "/home/yuxuanli/.conan/data/thrift/0.9.3/sztomi/testing/export/conanfile.py", line 110, in build
thrift/0.9.3@sztomi/testing: WARN: ACLOCAL_PATH = '$ACLOCAL_PATH:{}'.format(self.deps_env_info['pkg-config'].path[1])
thrift/0.9.3@sztomi/testing: WARN: IndexError: list index out of range
ERROR: thrift: list index out of range

Could you please give me more instructions on how to run your code? Thanks.
Also, I noticed that there are some updates to the get_blob() on master, are they related to the issue here?

Contributor

lyuxuan commented Mar 15, 2017

@sztomi sorry about the late reply. I cloned your code on a VM and tried to reproduce it, but I have some issues with conan.
Running the "conan install .. --build missing", I got the error shown below:
ERROR: pkg-config/0.29.1@sztomi/testing: Error in source() method, line 21
tools.download(tarball_url, tgz)
Error downloading file https://pkgconfig.freedesktop.org/releases/pkg-config-0.29.1.tar.gz: 'hostname 'pkgconfig.freedesktop.org' doesn't match either of 'distributions.freedesktop.org', 'farsight.freedesktop.org', 'fontconfig.freedesktop.org', 'fontconfig.org', 'freedesktop.org', 'geoclue.freedesktop.org', 'secure.freedesktop.org', 'www.fontconfig.org', 'www.freedesktop.org''

and then I added verify=false to download(). New error appeared:
thrift/0.9.3@sztomi/testing: ERROR: Package '5803b7ddb711007bf721b44c0023e9272981760a' build failed
thrift/0.9.3@sztomi/testing: WARN: Build folder /home/yuxuanli/.conan/data/thrift/0.9.3/sztomi/testing/build/5803b7ddb711007bf721b44c0023e9272981760a
thrift/0.9.3@sztomi/testing: WARN: File "/home/yuxuanli/.conan/data/thrift/0.9.3/sztomi/testing/export/conanfile.py", line 110, in build
thrift/0.9.3@sztomi/testing: WARN: ACLOCAL_PATH = '$ACLOCAL_PATH:{}'.format(self.deps_env_info['pkg-config'].path[1])
thrift/0.9.3@sztomi/testing: WARN: IndexError: list index out of range
ERROR: thrift: list index out of range

Could you please give me more instructions on how to run your code? Thanks.
Also, I noticed that there are some updates to the get_blob() on master, are they related to the issue here?

@sztomi

This comment has been minimized.

Show comment
Hide comment
@sztomi

sztomi Mar 15, 2017

@lyuxuan so apparently the last conan update broke some of the packages. That said, it looks like you are building the master branch. Could you try building only the repro branch (grpc_high_memory_repro)? The problematic benchmarks are disabled on the master branch anyway. If it still fails, you could try conan 0.18, because that's the version I used. I'll eventually get around to fix my packages but it'll take time.

sztomi commented Mar 15, 2017

@lyuxuan so apparently the last conan update broke some of the packages. That said, it looks like you are building the master branch. Could you try building only the repro branch (grpc_high_memory_repro)? The problematic benchmarks are disabled on the master branch anyway. If it still fails, you could try conan 0.18, because that's the version I used. I'll eventually get around to fix my packages but it'll take time.

@lyuxuan

This comment has been minimized.

Show comment
Hide comment
@lyuxuan

lyuxuan Apr 12, 2017

Contributor

Hi @sztomi could you please update your grpc version? I tried your benchmark with grpc v1.2.3, no out of memory issue showed up.

Contributor

lyuxuan commented Apr 12, 2017

Hi @sztomi could you please update your grpc version? I tried your benchmark with grpc v1.2.3, no out of memory issue showed up.

@sztomi

This comment has been minimized.

Show comment
Hide comment
@sztomi

sztomi Apr 13, 2017

sztomi commented Apr 13, 2017

@lyuxuan lyuxuan closed this May 8, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment