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

[AMORO-1865]build a rule of relationship between table and resource group #3300

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Aireed
Copy link
Contributor

@Aireed Aireed commented Oct 28, 2024

Why are the changes needed?

Close #1865.

Brief change log

  • Front-end:
    • Configure by adding rules in the properties of optimize-group with key 'match.rules'

image

  • Backend:

      1. When creating a group, rule conflict detection will be performed.
        And traverse the table corresponding to the catalog default value of the current optimize-group to see if it matches the current rule and assign the value.
      1. When updating the optimize-group rules, rule conflict detection will be performed.
        1. First, the table under the current rule will be judged. If the rule no longer matches after the rule is changed, the default group of the catalog to which the table belongs will be used.
        1. Traverse the table corresponding to the catalog default value of the current optimize-group to see if it matches the current rule and assign the value.
      1. when deleting the group
        1. Configure the tables under the group to the default group of the catalog to which they belong.
          When the group of the above table is changed, the tableConfigureChange responsibility chain call of the table will be triggered.
      1. When synchronizing the external catalog, the group will be automatically matched according to regular rules.
      1. TableRuntimeRefreshExecutor no longer pays attention to optimize-group when refreshing.
  • Special note
    We support manually adding the table name to the rules of a group, which is equivalent to the original manual configuration of the group. Because in our practice, there are some situations that require manual specification.

How was this patch tested?

  • Add some test cases that check the changes thoroughly including negative and positive cases if possible

  • Add screenshots for manual tests if appropriate

  • Run test locally before making a pull request

Documentation

  • Does this pull request introduce a new feature? (yes / no)
  • If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)

@@ -123,6 +123,10 @@ public String toString() {
return String.format("%s.%s.%s(tableId=%d)", catalog, database, tableName, id);
}

public String getTableFullName() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this method nessessary? How about invoke getIdentifier().toString() to get full name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this method nessessary? How about invoke getIdentifier().toString() to get full name?

getIdentifier().toString() returns TableIdentifier[catalog.db.table], So it's not suitable.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, It not org.apache.amoro.table.TableIdentifier. I perfer use the following method, WDYT?

  public org.apache.amoro.table.TableIdentifier getTableIdentifier() {
    return org.apache.amoro.table.TableIdentifier.of(catalog, database, tableName);
  }

.collect(Collectors.toList());

final String tableName =
String.format(
Copy link
Contributor

Choose a reason for hiding this comment

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

tableIdentifier.toString()

@baiyangtx
Copy link
Contributor

This implementation method will be contrary to the current advanced issue of splitting the rest service and the scheduling service. It makes the update of the group have to occur at the master node.

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.

[Improvement]: Build a rule of relationship between table and optimizer/resource group
3 participants