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

Proposal: Adapt Multiple Type of Database for Harbor #188

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

JinXingYoung
Copy link

@JinXingYoung JinXingYoung commented Mar 8, 2022

Signed-off-by: JinXingYoung yvonnexyang@tencent.com
Follow-up issue: goharbor/harbor#6534

@JinXingYoung JinXingYoung force-pushed the support-multidb branch 2 times, most recently from b3e1c80 to d6a311e Compare March 8, 2022 11:55
@JinXingYoung JinXingYoung changed the title Adapt Multiple Type of Database for Harbor Proposal: Adapt Multiple Type of Database for Harbor Mar 8, 2022
tianon
tianon previously approved these changes Mar 8, 2022
Copy link
Member

@tianon tianon left a comment

Choose a reason for hiding this comment

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

Very nice, I like it!

The biggest reason I would like to use MySQL instead of PostgreSQL is that the upgrade process (especially across major versions) is a lot more straightforward in MySQL than it is in PostgreSQL. 😇

@steven-zou
Copy link
Contributor

The proposal is really cool and I believe there are many community users expecting it!

Back to the proposal content, as there are multiple DB supported, do we need to show the DB configurations in the system settings page of the web console?

For abstracting the DB differences, have you guys evaluated any DB ORM library like gorm?

@JinXingYoung

@hyy0322
Copy link
Contributor

hyy0322 commented Mar 9, 2022

Back to the proposal content, as there are multiple DB supported, do we need to show the DB configurations in the system settings page of the web console?

I think we don't need to show DB configurations in web console. It is a deployment configuration.

For abstracting the DB differences, have you guys evaluated any DB ORM library like gorm?

No, still use beego orm. You mean we can use gorm?

@steven-zou

@JinXingYoung
Copy link
Author

Back to the proposal content, as there are multiple DB supported, do we need to show the DB configurations in the system settings page of the web console?

I think DB configurations can be shown in web console, but can't edit it in the web.

For abstracting the DB differences, have you guys evaluated any DB ORM library like gorm?

As @hyy0322 said, we still use beego orm. We can have a discussion about evaluating other DB ORM library like gorm later.

@OrlinVasilev
Copy link
Member

@wy65701436 please add your comments here that we just discussed on the Community Meeting! Thank you!

@nyarly
Copy link

nyarly commented Mar 9, 2022

I'm disappointed to see databases other than PostgreSQL and MySQL being listed as non-goals. For more than a year I've been watching issues related to making CockroachDB available. It's strikes me as unfortunate for a CNCF project to rely on a non-cloud-native datastore.

@OrlinVasilev
Copy link
Member

@nyarly - Will be great if you can submit such proposal and contribute on it :) Do you have resources to work on proposal for adopting CockroachDB? Will be super happy to see that effort going as well!

@nyarly
Copy link

nyarly commented Mar 9, 2022

@OrlinVasilev I've been doing the exploratory work to do that, but I'm blocked by dependency issues (c.f. goharbor/harbor#16420)

In the meantime, could the scope of the non-intentions be made clear? That this is restricted to this effort, but that other RDBMSes aren't out of scope for the Harbor project?

@tianon
Copy link
Member

tianon commented Mar 9, 2022

I can't speak for @JinXingYoung (so please correct me if this wasn't your intention), but I think the proposal as it reads currently is reasonably clear that it's the goal of this proposal to support MySQL(-like) but that others would also be interesting if we had folks willing to do the work (and maintain it over time):

Propose to support other databases in Harbor, the first step is to support MySQL/MariaDB.

(Quoted from the intro paragraph of the proposal.)

I don't think it hurts to be more clear about the scope of the non-goals section being just this proposal and not the entire project (and your work towards other databases like CockroachDB is definitely appreciated! ❤️), but I don't think that's necessarily unclear as-is either. 👍

@hyy0322
Copy link
Contributor

hyy0322 commented Mar 10, 2022

I can't speak for @JinXingYoung (so please correct me if this wasn't your intention), but I think the proposal as it reads currently is reasonably clear that it's the goal of this proposal to support MySQL(-like) but that others would also be interesting if we had folks willing to do the work (and maintain it over time):

Propose to support other databases in Harbor, the first step is to support MySQL/MariaDB.

(Quoted from the intro paragraph of the proposal.)

I don't think it hurts to be more clear about the scope of the non-goals section being just this proposal and not the entire project (and your work towards other databases like CockroachDB is definitely appreciated! ❤️), but I don't think that's necessarily unclear as-is either. 👍

@nyarly As @nyarly explained, you can join us to support CockroachDB. ❤️

@JinXingYoung
Copy link
Author

In the meantime, could the scope of the non-intentions be made clear? That this is restricted to this effort, but that other RDBMSes aren't out of scope for the Harbor project?

Yes, our intention is to support mysql and Mariadb this time, but also to help more databases can be supported. I'll update non-goals clearly to declare that other RDBMSes are welcomed to be supported in as well.

@nyarly
Copy link

nyarly commented Mar 10, 2022

@JinXingYoung That's great! I've been doing initial work to support Cockroach, and it may be worth discussing overlap; I suspect we're about as much out of TZ sync as we could be, but async communication would work well. I don't want to hijack comments on your proposal arranging that. Can you let me know what would be most convenient for you?

Staying on topic, I think it will eventually be the case that Cockroach will need a slightly different DB initialization (like where Harbor currently passes migrator the string "postgresql" it needs to pass "cockroachdb"), but that the DB driver should be the same for normal operations.

I would love to keep that variation part of the work you're proposing and will be following it avidly.

@JinXingYoung
Copy link
Author

@JinXingYoung That's great! I've been doing initial work to support Cockroach, and it may be worth discussing overlap; I suspect we're about as much out of TZ sync as we could be, but async communication would work well. I don't want to hijack comments on your proposal arranging that. Can you let me know what would be most convenient for you?

Staying on topic, I think it will eventually be the case that Cockroach will need a slightly different DB initialization (like where Harbor currently passes migrator the string "postgresql" it needs to pass "cockroachdb"), but that the DB driver should be the same for normal operations.

I would love to keep that variation part of the work you're proposing and will be following it avidly.

I quite agree with you and we'd like to invite you to join our wg. Common issues such as parameter passing and database initialization can use the same logic, and other compatibility issues can also be discussed.

We mostly use wechat. You can contact me through email first, so that we can discuss the appropriate way of communication. My email is yvonnexyang@tencent.com. Maybe our TZ is 16 hours behind you, but as you said, async communication would work well.❤️

@JinXingYoung
Copy link
Author

JinXingYoung commented Mar 21, 2022

A meeting will be held via slack channel on March 22 at 10:30AM GMT. Welcome to join us.

Meeting Agenda:

  1. Orm libraries to choose. Beego ORM, GORM, or see if there are any other libraries available
  2. Mysql driver does not need to implement all DAO interface, Orm functions can be generic. We only need to change the parts that do not fit, such as the SQL statement implementation incompatible parts.
  3. Data migration tool

Other topics can also be discussed.

To join slack channel under CNCF:

  1. To access the CNCF slack, you need an invitation which can be obtained at https://slack.cncf.io/.
    
  2. Join into the harbor-multi-database-workgroup channel (https://cloud-native.slack.com/archives/C037VBX8U57)
    

@OrlinVasilev
Copy link
Member

@nyarly ^^ hope you can join and if anyone from the maintainers can join as well will be great! @tianon @steven-zou @Vad1mo

@Vad1mo
Copy link
Member

Vad1mo commented Mar 22, 2022

😞 sorry didn't see that

@JinXingYoung
Copy link
Author

JinXingYoung commented Dec 23, 2022

@wy65701436 @tianon @chlins Please help review this PR, thanks ^^

Copy link
Member

@Vad1mo Vad1mo left a comment

Choose a reason for hiding this comment

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

  • Remove Notary support, as notary is already deprecated.
  • I would also suggest removing the migration support, it is not something Harbor should support and maintain.

@JinXingYoung
Copy link
Author

  • Remove Notary support, as notary is already deprecated.
  • I would also suggest removing the migration support, it is not something Harbor should support and maintain.

@Vad1mo Sorry for the late reply, I have been unwell recently.

Agreed to remove the notary part.

Considering that there may be a need to migrate data from pg to mysql, we provide a migration tool for the data migration part. I think some explanation can be preserved. We can add an explanation that users can choose to use this solution or migrate the data themselves. Is it ok?

Signed-off-by: JinXingYoung <yvonnexyang@tencent.com>
@sagikazarmark
Copy link

I can't see a resolution on the ORM discussion in the thread here, but I'd like to add Ent for consideration. IMO it just beats every other option right now.

Regardless of which ORM you decide to go with (if any):

  • It can make adding support for additional DB engines easier (eg. Ent already has preview support for CockroachDB: https://entgo.io/docs/dialects)
  • It makes testing easier
  • It would probably make sense to hide the ORM behind an interface, so adding support for other DB engines is still possible

@JinXingYoung
Copy link
Author

I can't see a resolution on the ORM discussion in the thread here, but I'd like to add Ent for consideration. IMO it just beats every other option right now.

This issue has been discussed in the working group before, and ent is the research object, but this revision does not switch the orm library. Later tasks will replace the orm library.

@sagikazarmark
Copy link

Thanks for the clarification.

@JinXingYoung
Copy link
Author

  • Remove Notary support, as notary is already deprecated.
  • I would also suggest removing the migration support, it is not something Harbor should support and maintain.

@Vad1mo Please help me to see if this is OK, thank you.

  • Notary already removed.
  • Adding an explanation that users can choose to use this solution or migrate the data themselves. We had planned to push the migration tool code to the harbor contrib folder, so I thought it should be included in the proposal. Or we put the migration tool to a new repository?

@Vad1mo
Copy link
Member

Vad1mo commented Feb 22, 2023

  • Adding an explanation that users can choose to use this solution or migrate the data themselves. We had planned to push the migration tool code to the harbor contrib folder, so I thought it should be included in the proposal. Or we put the migration tool to a new repository?
    I favor a solution that has the lowest amount of maintenance in the future and is decoupled from the main product.

My main concern here is that we create a new tool that we have to maintain in the future that has nothing to do with the core product. So eventually, we will end up with yet another DB migration tool that needs maintenance but does not add any more value than all the other tools out there. Maybe it would be better to create a script using an existing DB migration tool?

@JinXingYoung
Copy link
Author

JinXingYoung commented Feb 22, 2023

I favor a solution that has the lowest amount of maintenance in the future and is decoupled from the main product.

My main concern here is that we create a new tool that we have to maintain in the future that has nothing to do with the core product. So eventually, we will end up with yet another DB migration tool that needs maintenance but does not add any more value than all the other tools out there. Maybe it would be better to create a script using an existing DB migration tool?

@Vad1mo We discussed the form of data migration tool with community members before, and the conclusion is that Harbor can support more database types in the future. In this time, migration tool development defines a basic framework and after new database types are added, new data migration capabilities can be developed directly based on this framework.
cc @wy65701436 @chlins @hyy0322

@JinXingYoung
Copy link
Author

@Vad1mo We discussed the form of data migration tool with community members before, and the conclusion is that Harbor can support more database types in the future. In this time, migration tool development defines a basic framework and after new database types are added, new data migration capabilities can be developed directly based on this framework. cc @wy65701436 @chlins @hyy0322

@Vad1mo Please help to see if this solution is OK? We want to define a general interface so that it can be extended to other db types in the future.

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.

9 participants