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

[Feature][Connector-V2] Support multi-table sink feature for HBase #7169

Merged
merged 23 commits into from
Aug 30, 2024

Conversation

BruceWong96
Copy link
Contributor

Purpose of this pull request

[Feature][HBase] Support multi-table sink feature #5652

Does this PR introduce any user-facing change?

How was this patch tested?

new e2e test

Check list

@davidzollo davidzollo added the First-time contributor First-time contributor label Jul 11, 2024
@BruceWong96
Copy link
Contributor Author

@Hisoka-X
@hailin0
Hi, PTAL.
Thanks.

@Hisoka-X
Copy link
Member

Thanks @BruceWong96 for contribute this feature! Could you follow the guide to open github action on your fork repository? https://github.com/apache/seatunnel/pull/7169/checks?check_run_id=27312810188

@BruceWong96
Copy link
Contributor Author

Thanks @BruceWong96 for contribute this feature! Could you follow the guide to open github action on your fork repository? https://github.com/apache/seatunnel/pull/7169/checks?check_run_id=27312810188

OK

@BruceWong96
Copy link
Contributor Author

@Hisoka-X
Up to now, Do I need to address JDK 11 compatibility issues ?
Looking forward to your reply !

@Hisoka-X
Copy link
Member

@Hisoka-X Up to now, Do I need to address JDK 11 compatibility issues ? Looking forward to your reply !

Yes. Before we merge PR, we should make sure all test case passed.
image

sink {
Hbase {
zookeeper_quorum = "hbase_e2e:2181"
table = "seatunnel_test"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does it support writing to multiple tables?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add test cases to write to tables with different structures

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does it support writing to multiple tables?

Maybe I have a problem with my understanding. Is it many-to-many that we're going to end up with ?

Copy link
Member

@hailin0 hailin0 Jul 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BruceWong96

You decide how to design this function.

e.g. multiple tables as input upstream:

source {
  FakeSource {
    tables_configs = [
      {
          schema {
            table = "table-11111"
            fields {
                id = string
                age = int
            }
          }
      },
      {
          schema {
            table = "table-2222"
            fields {
               f1 = boolean
               f2 = boolean
               f3 = tinyint
            }
          }
      }
    ]
  }
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BruceWong96

You decide how to design this function.

e.g. multiple tables as input upstream:

source {
  FakeSource {
    tables_configs = [
      {
          schema {
            table = "table-11111"
            fields {
                id = string
                age = int
            }
          }
      },
      {
          schema {
            table = "table-2222"
            fields {
               f1 = boolean
               f2 = boolean
               f3 = tinyint
            }
          }
      }
    ]
  }
}
env {
  parallelism = 1
  job.mode = "BATCH"
}


source {
  FakeSource {
    tables_configs = [
      {
          schema {
            table = "table-11111"
            fields {
                id = string
                age = int
            }
          }
      },
      {
          schema {
            table = "table-2222"
            fields {
               f1 = boolean
               f2 = boolean
               f3 = tinyint
            }
          }
      }
    ]
  }
}

sink {
  Hbase {
    zookeeper_quorum = "hadoop001:2181,hadoop002:2181,hadoop003:2181"
    tables_configs = [
      {
        source_table_name = "table-11111"
        table = "hbase_table_1"
        rowkey_column = ["id"]
        family_name {
          id = "cf1"
          age = "cf1"
        }
      },
      {
        source_table_name = "table-2222"
        table = "hbase_table_2"
        rowkey_column = ["f1"]
        family_name {
          f1 = "cf2"
          f2 = "cf2"
          f3 = "cf2"
        }
      }
    ]
  }
}

Please check whether this configuration meets the requirements. If so, I will develop according to this configuration.
Thanks.

Copy link
Member

@hailin0 hailin0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update docs

sink {
Hbase {
zookeeper_quorum = "hbase_e2e:2181"
table = "seatunnel_test"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add test cases to write to tables with different structures

@hailin0
Copy link
Member

hailin0 commented Jul 22, 2024

@BruceWong96 I found a duplicate PR, here #6969, This is the correct way to code, you can use it as a reference for learning

@TaoZex @BruceWong96 Are you willing to collaborate with each other?

@BruceWong96
Copy link
Contributor Author

@BruceWong96 I found a duplicate PR, here #6969, This is the correct way to code, you can use it as a reference for learning

@TaoZex @BruceWong96 Are you willing to collaborate with each other?

OK , I'm going to learn about it. So you can close this PR. Thanks.

@TaoZex
Copy link
Contributor

TaoZex commented Jul 23, 2024

@BruceWong96 I found a duplicate PR, here #6969, This is the correct way to code, you can use it as a reference for learning
@TaoZex @BruceWong96 Are you willing to collaborate with each other?

OK , I'm going to learn about it. So you can close this PR. Thanks.

I close that pr, you can use it as a reference to try to implement it. Feel free to contact me if you have any questions.

@BruceWong96
Copy link
Contributor Author

@BruceWong96 I found a duplicate PR, here #6969, This is the correct way to code, you can use it as a reference for learning
@TaoZex @BruceWong96 Are you willing to collaborate with each other?

OK , I'm going to learn about it. So you can close this PR. Thanks.

I close that pr, you can use it as a reference to try to implement it. Feel free to contact me if you have any questions.

Thanks a lot . I will commit the code at a later time.

@hailin0
Copy link
Member

hailin0 commented Aug 9, 2024

hi @BruceWong96
Is there any progress on this task?

@BruceWong96
Copy link
Contributor Author

hi @BruceWong96 Is there any progress on this task?

Yes, I am doing some local tests, sorry for the delay, I will update the code soon.

Thanks.

@BruceWong96 BruceWong96 force-pushed the multi-table-sink-hbase branch 2 times, most recently from 8b7c393 to 75b3fc1 Compare August 12, 2024 11:15
@BruceWong96 BruceWong96 marked this pull request as draft August 14, 2024 05:46
@BruceWong96 BruceWong96 marked this pull request as ready for review August 14, 2024 09:58
@BruceWong96
Copy link
Contributor Author

@hailin0 PTAL.
Thanks.

@BruceWong96
Copy link
Contributor Author

@Hisoka-X PTAL.
Thanks.

@BruceWong96
Copy link
Contributor Author

@hailin0 PTAL.
Thanks.

Copy link
Member

@Hisoka-X Hisoka-X left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM if ci passes. Thanks @BruceWong96 !

@hailin0 hailin0 merged commit 025fa3b into apache:dev Aug 30, 2024
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants