Basic implementation of bigtable::Table::ReadModifyWrite()

[coryan]

This depends on #300, it must include (a potentially super trivial) unit test.

[hemant-qlogic]

Understanding:

To implement ReadModifyWrite function I was thinking two ways.

1. To implement this function inside table.h
2. To implement this function inside TableAdmin.h

table.h =>
As you have mentioned at multiple places that the function ReadModifyWrite should be inside table.h.
As per my understanding inside table.h, row_reader.h => v2::ReadRowsRequest is used to retrieve the rows.
So if I follow this way then I have to to create new row_modifier.h
which will internally use v2::ReadModifyWriteRowRequest, where I can
add ReadModifyWriteRules.

TableAdmin.h =>
As per your suggestions in my earlier queries, the function ReadModifyWrite can use the CallWithoutRetry in a similar way TableAdmin::ModifyColumnFamilies.
This class is using the protos from bigtable_table_admin.pb.h. Inside this file there
is no class for api ReadModifyWrite functionality.

Third way I can think of:
I should implement the api inside table.h and use the v2::ReadModifyWriteRowRequest with CallWithoutRetry.
Please let me know which way is correct or my direction is wrong?

ReadModifyWrite function declaration.
void ReadModifyWrite(std::string row_key, bigtable::ReadModifyWriteRule&&... rules);
is it ok?

[coryan]

Understanding:

To implement ReadModifyWrite function I was thinking two ways.

1. To implement this function inside table.h

The issue explicitly spells out that this is to be a member function of bigtable::Table, therefore it must be declared in table.h, if it is a template function (looks like it might be) at least most of the implementation should be the the .h file, but we should try to refactor as much as possible to the .cc file.

2. To implement this function inside TableAdmin.h

table.h =>
As you have mentioned at multiple places that the function ReadModifyWrite should be inside table.h.
As per my understanding inside table.h, row_reader.h => v2::ReadRowsRequest is used to retrieve the
rows.
So if I follow this way then I have to to create new row_modifier.h
which will internally use v2::ReadModifyWriteRowRequest, where I can
add ReadModifyWriteRules.

While that is what we did for row reader, because the state machine is complex, for other functions (e.g. Table::Apply()) the implementation does not require a separate class, it is a relatively small function.

TableAdmin.h =>
As per your suggestions in my earlier queries, the function ReadModifyWrite can use the
CallWithoutRetry in a similar way TableAdmin::ModifyColumnFamilies.
This class is using the protos from bigtable_table_admin.pb.h.

Where? bigtable::UnaryRpcUtils<>::CallWithoutRetry<>() is a template function, it takes the types as parameters.

Inside this file there
is no class for api ReadModifyWrite functionality.

I am not sure what file you are referring to: unary_rpc_utils.h? Certainly, but note that there is no TableAdmin functionality in there either. in table_admin.h or its .cc counterpart? True, but that is because it does not belong there and has not been written yet.

ReadModifyWrite function declaration.
void ReadModifyWrite(std::string row_key, bigtable::ReadModifyWriteRule&&... rules);

Well, you can not declare a variadic function with a concrete list of types (I think) so it should be:

template<typename... Args>
void ReadModifyWrite(std::string row_key, Args&&... rules);

But it makes no sense to call the function with 0 arguments, so I think it should be something like:

template <typename... Args>
void ReadModifyWrite(std::string row_key, bigtable::ReadModifyWriteRule rule, Args&&... rules)

And to clarify even further my notes about how UnaryRpcUtils applies, this is the skeleton of the function...

template <typename... Args>
void ReadModifyWrite(std::string row_key, bigtable::ReadModifyWriteRule rule, Args&&... rules) {
  using RpcUtils = bigtable::internal::UnaryRpcUtils<DataClient>;
  using StubType = RpcUtils::StubType;
  using btproto = google::bigtable::v2;

  btproto::ReadModifyWriteRequest request;
  request.set_table_name(table_name_);
  request.set_row_key(std::move(row_key));
  *request.add_rules() = std::move(rule);
  // TODO() - need to add any additional rules in the variadic argument list.

  // TODO() - the rest of the code can be refactored to the .cc file in a private member function.
  auto result = RpcUtils::CallWithoutRetry(*client_, rpc_retry_policy_->clone(),
                                    &StubType::ReadModifyWrite, request,
                                    "ReadModifyWrite()"); // TODO() - add more details to error message, table name and row are enough...
  Row row;
  // TODO() - convert the result into a Row object
  return row;
}

I hope that answers your questions, I should be available tomorrow morning for IM if that would help.