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

incompatible code generation for flatbuffer grpc service with c++ #5836

Closed
pavanbadugu opened this issue Mar 30, 2020 · 34 comments · Fixed by #6338
Closed

incompatible code generation for flatbuffer grpc service with c++ #5836

pavanbadugu opened this issue Mar 30, 2020 · 34 comments · Fixed by #6338

Comments

@pavanbadugu
Copy link

created a fbs file with a grpc service and executed
flatc --cpp --grpc log_query_service.fbs got the file generated but the header file and the cc file has compile time errors

grpc 1.27.3
flatbuffer 1.11.0

please let me know what version to use to resolve this issue
thank you

@aardappel
Copy link
Collaborator

What are the errors? Can you try with FlatBuffers 1.12 or master?

@pavanbadugu
Copy link
Author

error: 'grpc::ClientContext' has not been declared
virtual ::grpc::Status search_log(::grpc::ClientContext* context, const flatbuffers::grpc::Message& request, flatbuffers::grpc::Message* response) = 0;

@pavanbadugu
Copy link
Author

pavanbadugu commented Mar 30, 2020

yes i tried with grpc master and flatbuffer master
this is my cmake

grpc and flatbuffers are installed manually inside /usr/local/include dir

cmake_minimum_required(VERSION 3.15)
project(episilia_query)
set(CMAKE_CXX_STANDARD 14)
include(FetchContent)
set(FETCHCONTENT_QUIET OFF)
FetchContent_Declare(
googletest
GIT_REPOSITORY https://github.com/google/googletest.git
)
FetchContent_MakeAvailable(googletest)
#add glog
include_directories(
/usr/local/include
/episilia-common
)
enable_testing()
add_subdirectory(test)
add_subdirectory(lib)
add_executable(episilia_query main.cpp )
target_link_libraries(${PROJECT_NAME} grpc++ flatbuffers lib_episilia_query)

these are the following commands i used to install grpc
git clone -b v1.27.3 https://github.com/grpc/grpc.git
cd grpc && cmake && make && make install
same for flat buffers

@aardappel
Copy link
Collaborator

Hmm, that's a pretty core GRPC class, and the current docs still refer to it, so I don't think they renamed it or something.. does the test code you include the generated code in have a #include <grpc++/grpc++.h> ?

Does this compile? https://github.com/google/flatbuffers/tree/master/grpc/samples/greeter

@pavanbadugu
Copy link
Author

pavanbadugu commented Mar 30, 2020

no it does not build but the error has changed

In file included from /home/pavan/CLionProjects/episilia-query/lib/src/log_query_service.grpc.fb.cc:6:0:
/home/pavan/CLionProjects/episilia-query/lib/include/log_query_service.grpc.fb.h:21:7: error: using typedef-name ‘grpc::CompletionQueue’ after ‘class’
class CompletionQueue;

@pavanbadugu
Copy link
Author

solved it by removing those class declarations but still facing the error
error: no matching function for call to ‘grpc::SerializationTraits<flatbuffers::grpc::Message, void>::Deserialize(grpc::ByteBuffer*, flatbuffers::grpc::Message&)’
::grpc::SerializationTraits::Deserialize(&buf, request);
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~
In file included from /home/pavan/CLionProjects/episilia-query/lib/include/log_query_service.grpc.fb.h:8:0,
from /home/pavan/CLionProjects/episilia-query/lib/src/log_query_service.grpc.fb.cc:6:
/usr/local/include/flatbuffers/grpc.h:290:23: note: candidate: static grpc::Status grpc::SerializationTraits<flatbuffers::grpc::Message >::Deserialize(grpc_byte_buffer
, flatbuffers::grpc::Message) [with T = LogSearchResponse; grpc_byte_buffer = grpc_byte_buffer]
static grpc::Status Deserialize(grpc_byte_buffer buffer,
^~~~~~~~~~~
/usr/local/include/flatbuffers/grpc.h:290:23: note: no known conversion for argument 1 from ‘grpc::ByteBuffer
’ to ‘grpc_byte_buffer

In file included from /usr/local/include/grpc++/impl/codegen/method_handler_impl.h:26:0,
from /home/pavan/CLionProjects/episilia-query/lib/include/log_query_service.grpc.fb.h:12,
from /home/pavan/CLionProjects/episilia-query/lib/src/log_query_service.grpc.fb.cc:6:
/usr/local/include/grpcpp/impl/codegen/method_handler_impl.h: In instantiation of ‘void* grpc_impl::internal::RpcMethodHandler<ServiceType, RequestType, ResponseType>::Deserialize(grpc_call*, grpc_byte_buffer*, grpc::Status*, void**) [with ServiceType = QueryService::Service; RequestType = flatbuffers::grpc::Message; ResponseType = flatbuffers::grpc::Message; grpc_call = grpc_call; grpc_byte_buffer = grpc_byte_buffer]’:
/home/pavan/CLionProjects/episilia-query/lib/src/log_query_service.grpc.fb.cc:84:1: required from here
/usr/local/include/grpcpp/impl/codegen/method_handler_impl.h:102:62: error: no matching function for call to ‘grpc::SerializationTraits<flatbuffers::grpc::Message, void>::Deserialize(grpc::ByteBuffer*, flatbuffers::grpc::Message&)’
::grpc::SerializationTraits::Deserialize(&buf, request);
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~
In file included from /home/pavan/CLionProjects/episilia-query/lib/include/log_query_service.grpc.fb.h:8:0,
from /home/pavan/CLionProjects/episilia-query/lib/src/log_query_service.grpc.fb.cc:6:
/usr/local/include/flatbuffers/grpc.h:290:23: note: candidate: static grpc::Status grpc::SerializationTraits<flatbuffers::grpc::Message >::Deserialize(grpc_byte_buffer
, flatbuffers::grpc::Message*) [with T = LogSearchRequest; grpc_byte_buffer = grpc_byte_buffer]
static grpc::Status Deserialize(grpc_byte_buffer *buffer,

@pavanbadugu
Copy link
Author

i have added
#include <grpc++/grpc++.h> in the service_generated.h file that is imported in the service.fbs.h file
but facing the above mentioned error can any please help me on this

@aardappel
Copy link
Collaborator

does this fix it? #5843

@pavanbadugu
Copy link
Author

my issue is with c++ server code generation will check it although thank you @aardappel

@MarceColl
Copy link

MarceColl commented Apr 26, 2020

Having the same error here, following the flatbuffers gRPC tutorial code (greeter example) verbatim.

@pavanbadugu
Copy link
Author

pavanbadugu commented Apr 26, 2020

does this fix it? #5843

nope it did not @aardappel but it fixed for swift but c++ is not yet supported some namespace changes have been occurred in c++ grpc which results in some of the compile error with some classes not declared in scope

@pavanbadugu
Copy link
Author

Having the same error here, following the flatbuffers gRPC tutorial code (greeter example) verbatim.

i would suggest you to use grpc 16.1.1 which is last working version that is compatible with flatc grpc generated code

@MarceColl
Copy link

Having the same error here, following the flatbuffers gRPC tutorial code (greeter example) verbatim.

i would suggest you to use grpc 16.1.1 which is last working version that is compatible with flatc grpc generated code

Okay! thanks, will try :)

aardappel pushed a commit that referenced this issue Jun 26, 2020
flatbuffers project currently depends on an old version of GRPC library and has known issues working with the latest version (issue #5836). Therefore, a patch file is created and put under bazel/ directory before supporting the latest GRPC version.
@thomas-t1
Copy link

We've run into this incompatibility with recent gRPC versions as well. We tried using VCPKG to install and build both flatbuffers and gRPC, and they are incompatible.

I noticed the reference above to 1.16.1 (I'm assuming 16.1.1 is a typo), but the bazel file refers to 1.15.1, what would be the correct version of gRPC to use with flatbuffers?

@inobelar
Copy link
Contributor

inobelar commented Jul 22, 2020

Hello everyone!

I was able to successfully compile and run the greeter example (and few of self custom schemas) using the flatbuffers-1.12.0 with latest (built from sources) gRPC-1.30.2 with the next manual modifications:

1. Modification in generated schema (greeter.grpc.fb.h for example)

Before (originally generated code from flatc):

#include "greeter_generated.h"
#include "flatbuffers/grpc.h"

#include <grpc++/impl/codegen/async_stream.h>
#include <grpc++/impl/codegen/async_unary_call.h>
#include <grpc++/impl/codegen/method_handler_impl.h>
#include <grpc++/impl/codegen/proto_utils.h>
#include <grpc++/impl/codegen/rpc_method.h>
#include <grpc++/impl/codegen/service_type.h>
#include <grpc++/impl/codegen/status.h>
#include <grpc++/impl/codegen/stub_options.h>
#include <grpc++/impl/codegen/sync_stream.h>

namespace grpc {
class CompletionQueue;
class Channel;
class ServerCompletionQueue;
class ServerContext;
}  // namespace grpc

class Greeter final {
  // ... rest of the code ...

After (manually patched):

#include "greeter_generated.h"
#include "flatbuffers/grpc.h"

#include <grpc++/impl/codegen/async_stream.h>
#include <grpc++/impl/codegen/async_unary_call.h>
#include <grpc++/impl/codegen/method_handler_impl.h>
#include <grpc++/impl/codegen/proto_utils.h>
#include <grpc++/impl/codegen/rpc_method.h>
#include <grpc++/impl/codegen/service_type.h>
#include <grpc++/impl/codegen/status.h>
#include <grpc++/impl/codegen/stub_options.h>
#include <grpc++/impl/codegen/sync_stream.h>

// =============================================================================
#include <grpc++/impl/codegen/client_context.h>

namespace grpc {
  using CompletionQueue       = ::grpc_impl::CompletionQueue;
  using Channel               = ::grpc_impl::Channel;
  using ServerCompletionQueue = ::grpc_impl::ServerCompletionQueue;
  using ServerContext         = ::grpc_impl::ServerContext;

  namespace internal
  {
      template <class RequestType, class ResponseType>
      using StreamedUnaryHandler = ::grpc_impl::internal::StreamedUnaryHandler<RequestType, ResponseType>;

      template <class ServiceType, class RequestType, class ResponseType>
      using RpcMethodHandler = ::grpc_impl::internal::RpcMethodHandler<ServiceType, RequestType, ResponseType>;

      template <class ServiceType, class RequestType, class ResponseType>
      using ServerStreamingHandler = ::grpc_impl::internal::ServerStreamingHandler<ServiceType, RequestType, ResponseType>;

      template <class RequestType, class ResponseType>
      using SplitServerStreamingHandler = ::grpc_impl::internal::SplitServerStreamingHandler<RequestType, ResponseType>;
  } // namespace internal
}  // namespace grpc
// =============================================================================

class Greeter final {
  // ... rest of the code ...
2. Modification in flatbuffers/include/flatbuffers/grpc.h

Additional two SerializationTraits::Dersesrialize overloads, added after here (after static grpc::Status Deserialize(grpc_byte_buffer *buffer, flatbuffers::grpc::Message<T> *msg) due to same errors, which @pavanbadugu mentioned

template<class T> class SerializationTraits<flatbuffers::grpc::Message<T>> {
 public:
  // ... Serialize here ...

  // Deserialize by pulling the
  static grpc::Status Deserialize(grpc_byte_buffer *buffer,
                                  flatbuffers::grpc::Message<T> *msg) {
    // ... body ...
  }

  // Overload 1) grpc::ByteBuffer -> grpc::ByteBuffer::ByteBufferPointer
  static grpc::Status Deserialize(grpc::ByteBuffer* buffer,
                                  flatbuffers::grpc::Message<T> *msg)
  {
      return Deserialize(grpc::ByteBuffer::ByteBufferPointer(buffer), msg);
  }

  // Overload 2) grpc::ByteBuffer::ByteBufferPointer -> grpc_byte_buffer
  static grpc::Status Deserialize(grpc::ByteBuffer::ByteBufferPointer buffer_ptr, 
                                  flatbuffers::grpc::Message<T>* msg)
  {
      return Deserialize(buffer_ptr.operator grpc_byte_buffer*(), msg);
  }
};

Why this peaces of code not in Pull Request:

  • Have no idea, is it ok. :) Maybe, instead of creating grcp & grpc::internal namespaces (only for compatibility for the rest of generated code), flatc must already generate code with grpc_impl and grpc_impl::internal namespaces ?
  • gRPC uses typedefs instead using
  • Is additional overloads for SerializationTraits::Deserialized ok? Or something really important may be missed.
  • <grpc++/...> includes currently deprecated (but exists for backward compatibility). Maybe generated schema must contain <grpcpp/...> ?

Since I lack competence in these matters, I leave the decision for all of you, especially @aardappel

@aardappel
Copy link
Collaborator

@yaoshengzhe (who is currently fixing some of the gRPC integration).

@aashray
Copy link

aashray commented Aug 19, 2020

Having the same issues. @aardappel @yaoshengzhe I am currently trying to use gRPC with flatbuffers (C++) but looks like support is broken after gRPC 1.16.1 (discussed above). I am very new to flatbuffers, but can try to help if needed.
Also, would you be able to comment on the additional overloads for SerializationTraits::Deserialized mentioned above by @inobelar?

@joseprupi
Copy link

I was also having the same issue and when trying with 1.16.0 and compiling the example I get below error:

/usr/bin/ld: /tmp/ccLp62cl.o: in function RunServer()': server.cpp:(.text+0x11d): undefined reference to grpc_empty_slice'
/usr/bin/ld: server.cpp:(.text+0x1ba): undefined reference to grpc::ServerBuilder::ServerBuilder()' /usr/bin/ld: server.cpp:(.text+0x1c5): undefined reference to grpc::InsecureServerCredentials()'
/usr/bin/ld: server.cpp:(.text+0x1d5): undefined reference to grpc::ServerBuilder::AddListeningPort(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::shared_ptr<grpc::ServerCredentials>, int*)' /usr/bin/ld: server.cpp:(.text+0x20f): undefined reference to grpc::ServerBuilder::RegisterService(grpc::Service*)'

...

Is it also related to the versions? I found this issue #5099 but I am not able to make it work either

@joseprupi
Copy link

Also, is the intent of flatbuffers to give support to gRPC? I am thinking on using them on a long term project but this compatibility problems make me think it could make the maintenance hard in the future.

@aardappel
Copy link
Collaborator

@joseprupi the way it currently works (we share the code generator between FlatBuffers and upstream Protobuf) is very fragile, any changes in upstream code can break us. But there really also is no easier way to keep up to date with gRPC.

Usually these changes are small and easy to fix, but for some reason no-one on here has been able to make a PR for them. @yaoshengzhe ?

@joseprupi
Copy link

Thanks @aardappel .

I will try to take a look to it but sincerely I don't know where to start so some tips would be appreciated :)

@aardappel
Copy link
Collaborator

@joseprupi I'd start by trying to build the C++ test in https://github.com/google/flatbuffers/tree/master/grpc with the latest version of gRPC, and seeing what errors come out. They should ideally all be solved by changes in the FlatBuffers generators or runtime code. You may have to update the files in https://github.com/google/flatbuffers/tree/master/grpc/src/compiler with newer versions copied from gRPC. Runtime changes may have to be made in https://github.com/google/flatbuffers/blob/master/include/flatbuffers/grpc.h

@joseprupi
Copy link

Hello @aardappel I have created this PR #6143 if someone can take a look to it

@joseprupi
Copy link

@aardappel I've tried to solve some o the issues you mentioned and closed the PR. As far as I see there is no way to access the byte_buffer without copy and reading the comments it does not seem is going to happen soon.

grpc/grpc#20594
grpc/grpc#17603

@aardappel
Copy link
Collaborator

@joseprupi wow, surprised they apparently care more about their API design than zero-copy. Well, maybe that means we should put in your workaround until they fix their stuff... with a big warning sign? That way at least people can work with it.

@joseprupi
Copy link

@aardappel Well I won't share my opinion but that two projects like this from the same company don't work together is weird :)

For me the solution is modifying grpc to make it work as I think is a good stack but I will try to spend some more time with the PR to at least have an official way to use it if.

Also I think this should be reflected in the README and probably this blog post https://grpc.io/blog/grpc-flatbuffers/, for me is the first entry when searching for flatbuffers+grpc in google and can be confusing.

@thomas-t1
Copy link

I have to say I'm a bit disappointed of the attitude of the gRPC team in this particular case, breaking a working feature in a related and supposedly supported project and begin dogmatic about correcting it is not very... agile 👎 Especially considering that the API is semi-official in the sense that unit tests use it.

I'm guessing the gprc modification you are talking about @joseprupi, is the comment made here?grpc/grpc#20594 (comment)

Thanks for the PR btw!

@aardappel
Copy link
Collaborator

Yes, I agree that is weird and unfortunate. In large companies, different parts don't always work together as you'd expect. And are not equally supported.

Agree that be great if it could be fixed in gRPC. I can ping some people but wouldn't get my hopes up.

@joseprupi
Copy link

@aardappel I hope you did not take it in a bad way, it was just an observation as to me seems clear that grpc would want to have a non copy version of flatbuffers but those are your processes and obviously the interests are different. And btw thank you for your work with flatbuffers.

@thomas-t1 if I keep going with this stack I will try to expose the byte_buffer with the fewer lines of code possible so maybe something like this grpc/grpc#17603. It seems there have been several attempts to solve this problem before.

ivannp pushed a commit to ivannp/flatbuffers that referenced this issue Oct 2, 2020
flatbuffers project currently depends on an old version of GRPC library and has known issues working with the latest version (issue google#5836). Therefore, a patch file is created and put under bazel/ directory before supporting the latest GRPC version.
@pavanbadugu
Copy link
Author

pavanbadugu commented Oct 3, 2020

@aardappel cant we just make them as a plugin to proto compiler which generates grpc files for flatbuffer rather then flatc taking it

@aardappel
Copy link
Collaborator

@pavanbadugu that would require FlatBuffers users to translate their .fbs schema to .proto. Then it would require us to write this plugin, and maintain it, because now we have the reverse problem: FlatBuffer changes breaking this plugin.

@shawncao
Copy link

shawncao commented Mar 8, 2021

Yeah, this is annoying - I have tried a few times to upgrade GRPC version to latest, but failed every time, the current working integration for me is this (flatbuffers v1.12.0 + grpc v1.20.0) with one single manual fix:

https://github.com/varchar-io/nebula/blob/master/ext/Grpc_Ext.cmake#L62-L74

just sharing this working but ugly trick if you may find it useful.

This also means it has potential integration problems with integrating more library such as google-cloud-cpp who depends on grpc too, because of this, I even can't use VCPKG to manage them (flatbuffers, grpc), I wish this community (Google) can fix this scenario and maintain a good integration across these libraries. Thanks!

@katetsu
Copy link

katetsu commented Apr 30, 2021

any updates here?

I'm on FlatBuffers 1.20 and grace 1.37.1 and the problem still persists. I would be nice to be able to use gRPC with FlatBuffers....

@aardappel
Copy link
Collaborator

@katetsu this was merged, which fixes the C++ FlatBuffers gRPC support in a temporary way: #6338
That will be part of the 2.0 release which should be happening soon, or you could just work with latest master.
We're working on more permanent ways the two projects will fit together better :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants