Implement bigtable::Table::ReadModifyWriteRow

[coryan]

This might require implementing wrappers for ReadModifyWriteRule in the same style as the bigtable::Filter::* wrappers.

This issue should only be closed once the unit and integration tests are written as well as the implementation itself. If it is easier, break down the work in smaller issues before starting.

• Implement a wrapper for google.bigtable.v2.ReadModifyWriteRule #300: Implement a wrapper for google.bigtable.v2.ReadModifyWriteRule.
• Implement bigtable::ReadModifyWriteRule::AppendValue() #301: Implement bigtable::ReadModifyWriteRule::AppendValue().
• Implement bigtable::ReadModifyWriteRule::IncrementAmount() #302: Implement bigtable::ReadModifyWriteRule::IncrementAmount() constructor function and the corresponding unit test.
• Basic implementation of bigtable::Table::ReadModifyWrite() #303: Implement bigtable::Table::ReadModifyWrite() member function and a simple unit test using AppendValue().
• Complete unit tests for bigtable::Table::ReadModifyWrite() #304: Complete unit tests for bigtable::Table::ReadModifyWrite().
• Implement simple integration test for Table::ReadModifyWrite() #305: Implement an integration test with a simple AppendValue() rule.
• Complete the integration test for Table::ReadModifyWrite() #306: Complete the integration test for Table::ReadModifyWrite().

[coryan]

A few observations: we may want to create two separate member functions for this: IncrementValue() and AppendValue(). The API using separate functions is more natural:

class Table { public:
  Row IncrementValue(std::string row_key, std::string family_name, std::string column, std::int64_t value);
  Row AppendValue(std::string row_key, std::string family_name, std::string column, std::string value);

The disadvantage is, of course, that if we ever created a new branch in google.bigtable.v2.ReadModifyWriteRule then we would need to add a new interface.
I think this proposal is more idiomatic in C++, the alternative:

class Table { public:
  Row ReadModifyWrite(std::string row_key, ReadModifyWriteRule rule);
};

is workable, but a bit more awkward, contrast:

table->AppendValue("foo", "fam", "col", ";more-stuff-at-the-end");

vs.

table->ReadModifyWrite("foo", bigtable::AppendValue("fam", "col", ";more-stuff-at-the-end");

[coryan]

@hemant-qlogic Anything to report? Will you able to merge a PR before 2018-03-31? Remember to leave some days for review and fixes in your estimate.

[hemant-qlogic]

Nothing till now, Will able to merge it till sunday

[hemant-qlogic]

My Initial understanding about the point.

1. table.h => Add method ReadModifyWrite. It has additional rules based on google::bigtable::v2::ReadModifyWriteRule(IncreamentValue, AppendValue).
2. row_modifier.h ==> Add new class row_modifier (similar as row_reader), which will initiate request of form google::bigtable::v2::ReadModifyWriteRowRequest(bigtable.pb.h) along with the ReadWriteModifyRule
3. Add test cases for row_modifier.h unit test cases.
4. Integration Test cases in data_integration_test.cc for ReadModifyWrite function.
5. ReadModifyWrite should run on a single row?
6. Only one rule from ReadModifyWrite should apply for single request.
   Is it correct?

[coryan]

1. table.h => Add method ReadModifyWrite. It has additional rules based on
   google::bigtable::v2::ReadModifyWriteRule(IncreamentValue, AppendValue).

That sounds reasonable, can you describe the prototype for this ReadModifyWrite member function?

2. row_modifier.h ==> Add new class row_modifier (similar as row_reader), which will initiate request
   of form google::bigtable::v2::ReadModifyWriteRowRequest(bigtable.pb.h) along with the
   ReadWriteModifyRule

It is possible I am missing something here. To me, it looks like RowReader has a lot of functionality to parse the incoming stream, reconstruct the rows, and retry failed RPCs. I also looks to me that the member function you need here is more similar to TableAdmin::ModifyColumnFamilies(), it can be refactored to a call into RpcUtils::CallWithoutRetry. Maybe you can explain why they need the additional class in more detail?

3. Add test cases for row_modifier.h unit test cases.

If we write the class then we certainly need unit tests for it.

Integration Test cases in data_integration_test.cc for ReadModifyWrite function.

Yes, we will need that.

5. ReadModifyWrite should run on a single row?

That is correct, the gRPC API is defined here:

https://github.com/googleapis/googleapis/blob/92f10d7033c6fa36e1a5a369ab5aa8bafd564009/google/bigtable/v2/bigtable.proto#L304

and it takes a single row.

6. Only one rule from ReadModifyWrite should apply for single request.
   Is it correct?

Unfortunately not, I had not noticed this, but multiple rules apply to each request:
https://github.com/googleapis/googleapis/blob/92f10d7033c6fa36e1a5a369ab5aa8bafd564009/google/bigtable/v2/bigtable.proto#L317

[coryan]

Done.