[GOBBLIN-1737] Fix bug when using mysql user quota manager #3595
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3595 +/- ##
============================================
+ Coverage 46.88% 46.89% +0.01%
- Complexity 10672 10676 +4
============================================
Files 2120 2120
Lines 83064 83075 +11
Branches 9250 9254 +4
============================================
+ Hits 38942 38961 +19
+ Misses 40554 40545 -9
- Partials 3568 3569 +1 ☔ View full report in Codecov by Sentry. |
umustafi
approved these changes
Nov 4, 2022
| @@ -295,7 +302,8 @@ public MysqlQuotaStore(BasicDataSource dataSource, String tableName) | |||
| DECREASE_FLOWGROUP_COUNT_SQL = "UPDATE " + tableName + " SET flowgroup_count=flowgroup_count-1 WHERE name = ?"; | |||
| DELETE_USER_SQL = "DELETE FROM " + tableName + " WHERE name = ? AND user_count<1 AND flowgroup_count<1"; | |||
|
|
|||
| String createQuotaTable = "CREATE TABLE IF NOT EXISTS " + tableName + " (name VARCHAR(20) CHARACTER SET latin1 NOT NULL, " | |||
| //Increase the length of name as we include the executor uri in it | |||
| String createQuotaTable = "CREATE TABLE IF NOT EXISTS " + tableName + " (name VARCHAR(500) CHARACTER SET latin1 NOT NULL, " | |||
There was a problem hiding this comment.
anything else contained in table name? how long is executor uri max? It will be helpful to give idea of what table name will be
There was a problem hiding this comment.
Under name, it will be "user name for the quota" + "spec uri". I tried 200 and see it does not work so directly give 500
| @@ -390,7 +390,8 @@ public synchronized void setActive(boolean active) { | |||
|
|
|||
| this.dagManagerMetrics.activate(); | |||
|
|
|||
| UserQuotaManager quotaManager = new InMemoryUserQuotaManager(config); | |||
| UserQuotaManager quotaManager = GobblinConstructorUtils.invokeConstructor(UserQuotaManager.class, | |||
| ConfigUtils.getString(config, ServiceConfigKeys.QUOTA_MANAGER_CLASS, ServiceConfigKeys.DEFAULT_QUOTA_MANAGER), config); | |||
There was a problem hiding this comment.
do you see other types of quota manager besides in memory actually being used?
There was a problem hiding this comment.
In warm standby mode, we need use mysql based quota manager
phet
pushed a commit
to phet/gobblin
that referenced
this pull request
Dec 3, 2022
* address comments * use connectionmanager when httpclient is not cloesable * [GOBBLIN-1737] Fix bug when using mysql user quota manager * fix tests Co-authored-by: Zihan Li <[email protected]>
phet
added a commit
to phet/gobblin
that referenced
this pull request
Dec 5, 2022
* upstream/master: [GOBBLIN-1747] add job.name and job.id to kafka and compaction workunits (apache#3607) [GOBBLIN-1746] Add fs.uri to FsDatasetDescriptor to support copy between volumes in GaaS (apache#3605) [GOBBLIN-1743] Ensure GobblinTaskRunner works without Yarn use (apache#3602) [GOBBLIN-1745] Fix bug in SimpleKafkaSpecProducer (apache#3604) [GOBBLIN-1739]Define Datanodes and Dataset Descriptor for Iceberg (apache#3596) do not close DestinationDatasetHandlerService prematurely (apache#3601) [GOBBLIN-1720]Add ancestors owner permissions preservations for iceberg distcp (apache#3577) [HOTFIX] Fix checkstyleMain (apache#3600) [GOBBLIN-1736] Add metrics for change stream monitor and mysql quota manager (apache#3593) [GOBBLIN-1737] Fix bug when using mysql user quota manager (apache#3595) Correct a log line and GTE with currect number of total task count (apache#3591)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Dear Gobblin maintainers,
Please accept this PR. I understand that it will not be reviewed until I have checked off all the steps below!
JIRA
Description
Change the way we get config to to consistent with other mysql store
Only check quota in compiler when the flow compile successful and explain flag is not set
Change the way we initialize the quota Manager.
Tests
Deploy locally and verify the mysql quota manager works as expected
Commits