[FLINK-33208] Support the writable metadata timestamp

[Tan-JiaLiang]

What is the purpose of the change

Currently, the hbase sink does not support write data with timestamp, which may cause the data to be written out of order. I suggest to support the timestamp writable metadata for hbase connector so that we can set the timestamp when we writing.

Brief change log

• support the writable metadata timestamp

Verifying this change

This change is already covered by existing tests, such as

• org.apache.flink.connector.hbase1.HBaseConnectorITCase#testTableSinkWithTimestampMetadata
• org.apache.flink.connector.hbase2.HBaseConnectorITCase#testTableSinkWithTimestampMetadata

[MartijnVisser]

@ferenc-csaky can you also help with this one? I'll also ping you on some of the other HBase PRs 😅

[Tan-JiaLiang]

@MartijnVisser Thanks for all the reviewer request, @ferenc-csaky you can review when you are free, I'll always keep an eye on all my PRs until it finish, thank you master!

[MartijnVisser]

CC @ferenc-csaky

[ferenc-csaky]

I think handling WritableMetadata as an enum makes interactions with the class kinf of hard to read.

I also wondered about is it make sense to handle interactions with WritableMetadata as it has multiple different metadata, while we only have TIMESTAMP for now? I would introduce those changes when there are other metadata type(s), cause if we do not add any, we have unnecessary complexity.

I opened a PR to your fork where I created a TimestampMetadata class and simplified interactions, did some refactor to eliminate duplications.

PTAL

[Tan-JiaLiang]

I think handling WritableMetadata as an enum makes interactions with the class kinf of hard to read.

I didn't think much of it at first, I just followed the WritableMetadata design of flink-connector-kafka.. Because I think it's best to be consistent with these kinds of implementations. But I rethink about it, for now the connectors have split into two different projects, and your refactor is better IMO, so +1 for your suggestion.

I also wondered about is it make sense to handle interactions with WritableMetadata as it has multiple different metadata, while we only have TIMESTAMP for now?

Yes, there also FLINK-30460 should also be supported IMO. After this PR merged, I'll support it ASAP.

I cherry-pick your PR and do a little bit of optimization, PTAL.

And one more question, i want to keep your changes, but i want to squash all the commit in one too (maybe it can good for maintainer merge my PR), any suggestion? Seems git can not keep one commit two authors.

[ferenc-csaky]

And one more question, i want to keep your changes, but i want to squash all the commit in one too (maybe it can good for maintainer merge my PR), any suggestion? Seems git can not keep one commit two authors.

Feel free to squash the commit, if you put a co author line to the end of the commit msg that is perfect:

Co-authored-by: Ferenc Csaky <[email protected]>

[ferenc-csaky]

Thank you, LGTM!

[Tan-JiaLiang]

And one more question, i want to keep your changes, but i want to squash all the commit in one too (maybe it can good for maintainer merge my PR), any suggestion? Seems git can not keep one commit two authors.

Feel free to squash the commit, if you put a co author line to the end of the commit msg that is perfect:

Co-authored-by: Ferenc Csaky <[email protected]>

Nice! I got it.