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

Fix Couchbase regression #9448

Merged
merged 2 commits into from Jul 1, 2021
Merged

Conversation

akrantz01
Copy link
Contributor

Required for all PRs:

  • Updated associated README.md.
  • Wrote appropriate unit tests.

resolves #9416

Fixed an out-of-bounds panic that occurred when one of the bucket stats fields was missing in the response from Couchbase. A check to see if the array is nil was added to all bucket stats fields.

@akrantz01 akrantz01 added regression something that used to work, but is now broken fix pr to fix corresponding bug area/couchbase labels Jun 29, 2021
@telegraf-tiger telegraf-tiger bot added the feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin label Jun 29, 2021
@akrantz01
Copy link
Contributor Author

@Levarix would you be willing to test once the binaries are built?

@akrantz01 akrantz01 removed the feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin label Jun 29, 2021
Copy link
Contributor

@srebhan srebhan left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@srebhan srebhan self-assigned this Jun 30, 2021
@reimda reimda merged commit e0ac507 into influxdata:master Jul 1, 2021
reimda pushed a commit that referenced this pull request Jul 7, 2021
(cherry picked from commit e0ac507)
bhsu-ms pushed a commit to bhsu-ms/telegraf that referenced this pull request Jul 12, 2021
@Levarix
Copy link

Levarix commented Jul 13, 2021

@akrantz01 I've tested this, and now we have another huge problem. I applied a new version of telegraf in a production environment and telegraf processes consumes a lot of memory and don't purge it. Memory usage is still increasing until instance stopped work. It happens only with telegraf with couchbase plugin

image

Zrzut ekranu 2021-07-13 o 09 47 15
In the second screenshot, you can see memory usage increasing and after downgrade to 1.18.3 back to normal stable consumption

@srebhan
Copy link
Contributor

srebhan commented Jul 13, 2021

@Levarix can you please open an issue for this?

@akrantz01 the problem might be in

func (cb *Couchbase) queryDetailedBucketStats(server, bucket string, bucketStats *BucketStats) error {
	// Set up an HTTP request to get the complete set of bucket stats.
	req, err := http.NewRequest("GET", server+"/pools/default/buckets/"+bucket+"/stats?", nil)
	if err != nil {
		return err
	}

	r, err := client.Do(req)
	if err != nil {
		return err
	}

	defer r.Body.Close()

	return json.NewDecoder(r.Body).Decode(bucketStats)
}

as the final json.NewDecoder(r.Body).Decode(bucketStats) will only decode the first valid JSON object, so if therer are multiple JSONs sent, the decoder will not read all off r.Body and thus might leak memory. Not sure though but maybe it gives you a hint?

@Levarix
Copy link

Levarix commented Jul 13, 2021

Created @srebhan #9495

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/couchbase fix pr to fix corresponding bug regression something that used to work, but is now broken
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Telegraf 1.19.0 isn't working with couchbase in 6.0.5 build 3340
4 participants