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

Fixed lag between file-sd and dns-sd #933

Merged
merged 1 commit into from
Mar 20, 2019
Merged

Conversation

jojohappy
Copy link
Member

Signed-off-by: jojohappy sarahdj0917@gmail.com

Changes

fixes: #830

Update the addresses from file SD by using DNS SD resolver when file SD is updated if necessary at once.

PTAL @GiedriusS @R4scal

Verification

Signed-off-by: jojohappy <sarahdj0917@gmail.com>
Copy link
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

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

It's bit hacky, but I think I am OK with that. Checked and dnsProvider is goroutine-safe

@@ -355,6 +355,7 @@ func runQuery(
}
fileSDCache.Update(update)
stores.Update(ctxUpdate)
dnsProvider.Resolve(ctxUpdate, append(fileSDCache.Addresses(), storeAddrs...))
Copy link
Member

Choose a reason for hiding this comment

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

This is good but what happens if there are no file SD targets configured? We'd still have to wait, no?

Copy link
Member

Choose a reason for hiding this comment

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

There is another one done every X minutes below (:

Copy link
Member

Choose a reason for hiding this comment

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

It's not fixing general lag I guess, this PR fixes just the lag of FileSD vs dns.

Copy link
Member

Choose a reason for hiding this comment

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

Ah sorry, I only gave a quick glance and I haven't noticed that, and I thought we were fixing a different kind of problem.

Copy link
Member Author

@jojohappy jojohappy Mar 19, 2019

Choose a reason for hiding this comment

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

It's not fixing general lag I guess, this PR fixes just the lag of FileSD vs dns.

@bwplotka Could you explain what is the general lag?

It's bit hacky

Agree with you. I think it is a simple way to resolve the issue, and I don't find the general lag that you said.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, by general lag I meant the time between each DNS refresh (that is configured by flag). It's kind of flag as we don't watch for changes (as for DNS it's impossible), we poll every X seconds.

@bwplotka bwplotka merged commit 9f4fa5b into thanos-io:master Mar 20, 2019
@jojohappy jojohappy deleted the fix_830 branch August 21, 2019 02:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

query: 30 seconds lag of adding store to query storeset on start
3 participants