Skip to content

Cannot use the LibraryClient with the watcher #18

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

Closed
wangchen615 opened this issue Feb 25, 2021 · 1 comment · Fixed by #20
Closed

Cannot use the LibraryClient with the watcher #18

wangchen615 opened this issue Feb 25, 2021 · 1 comment · Fixed by #20
Assignees
Labels
bug Something isn't working help wanted Extra attention is needed question Further information is requested
Milestone

Comments

@wangchen615
Copy link
Collaborator

wangchen615 commented Feb 25, 2021

The current design of libraryClient in the release version includes watcher as follows:

// Client for Watcher APIs as a library
type libraryClient struct {
	fetcherClient watcher.MetricsProviderClient
	watcher       *watcher.Watcher
}

Watcher libraries should only be used for load-watcher. libraryClient should not include starting the watcher when the libraryClient is also used by scheduler plugins.

// Creates a new watcher client when using watcher as a library
func NewLibraryClient(opts watcher.MetricsProviderOpts) (Client, error) {
	var err error
	client := libraryClient{}
	switch opts.Name {
	case watcher.PromClientName:
		client.fetcherClient, err = metricsprovider.NewPromClient(opts)
	case watcher.SignalFxClientName:
		client.fetcherClient, err = metricsprovider.NewSignalFxClient(opts)
	default:
		client.fetcherClient, err = metricsprovider.NewMetricsServerClient()
	}
	if err != nil {
		return client, err
	}

	client.watcher = watcher.NewWatcher(client.fetcherClient)
	client.watcher.StartWatching()
	return client, nil
}

The LibraryClient failed to getLatestMetrics when using it in plugins without select {}.

@wangchen615 wangchen615 added help wanted Extra attention is needed question Further information is requested labels Feb 25, 2021
wangchen615 added a commit that referenced this issue Feb 25, 2021
@zorro786
Copy link
Contributor

Issue is with fetches below from windowWatcher() which run in separate go routine:

	for _, duration := range durations {
		go windowWatcher(duration)
	}

This may cause the metrics to be unavailable for a very short moment initially when using load watcher as a library.
To fix this, the first fetch should be made synchronously, and subsequent fetches should use goroutines. Thanks for reporting @wangchen615

@zorro786 zorro786 added this to the v0.1.1 milestone Sep 16, 2021
@zorro786 zorro786 added the bug Something isn't working label Sep 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants