Skip to content

Synchronize scheduled poll() calls in SQLMetadataRuleManager to prevent flakiness in SqlMetadataRuleManagerTest#6033

Merged
jihoonson merged 1 commit intoapache:masterfrom
metamx:SqlMetadataRuleManager-fix
Jul 24, 2018
Merged

Synchronize scheduled poll() calls in SQLMetadataRuleManager to prevent flakiness in SqlMetadataRuleManagerTest#6033
jihoonson merged 1 commit intoapache:masterfrom
metamx:SqlMetadataRuleManager-fix

Conversation

@leventov
Copy link
Copy Markdown
Member

This PR should fix #6028, however it doesn't address two other problems mentioned in that issue:

  • Possibility of a deadlock in a database when there is a query making an inner join of a table with itself, and a query dropping this table in parallel. I wonder if there is an SQL feature to "truly lock" a table to prevent this.
  • SQLMetadataRuleManager possibly mixing two abstractions, that should be in different classes.

@leventov leventov added the Bug label Jul 21, 2018
@leventov leventov requested a review from jihoonson July 21, 2018 19:11
*/
private long currentStartOrder = -1;
private ScheduledExecutorService exec = null;
private long retryStartTime = 0;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: looks that this can be a local variable.

Copy link
Copy Markdown
Contributor

@jihoonson jihoonson left a comment

Choose a reason for hiding this comment

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

LGTM

@jihoonson jihoonson merged commit 7d5eb0c into apache:master Jul 24, 2018
@leventov leventov deleted the SqlMetadataRuleManager-fix branch July 25, 2018 03:57
@leventov leventov added this to the 0.12.2 milestone Jul 25, 2018
@jihoonson
Copy link
Copy Markdown
Contributor

This is a regression bug and should be included in 0.12.2.

@jihoonson
Copy link
Copy Markdown
Contributor

NVM. It's already tagged.

jihoonson pushed a commit to jihoonson/druid that referenced this pull request Jul 27, 2018
jihoonson added a commit that referenced this pull request Jul 27, 2018
jihoonson pushed a commit to implydata/druid-public that referenced this pull request Jul 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Error in SqlMetadataRuleManagerTest

2 participants