Skip to content
This repository was archived by the owner on Apr 2, 2024. It is now read-only.

Conversation

harkishen
Copy link
Member

@harkishen harkishen commented Feb 21, 2022

Signed-off-by: Harkishen-Singh harkishensingh@hotmail.com

Description

Describe the big picture of your changes here to communicate to the maintainers why we should accept this pull request. If it fixes a bug or resolves a feature request, be sure to link to that issue.

Merge requirements

Please take into account the following non-code changes that you may need to make with your PR:

  • CHANGELOG entry for user-facing changes
  • Updated the relevant documentation

@harkishen harkishen requested a review from a team as a code owner February 21, 2022 10:29
@harkishen harkishen self-assigned this Feb 21, 2022
samplesCopierChannelToMonitor chan readRequest
// Declare and register here since we need readRequest struct which should not be defined in
// pgmodel/metric.
ingestorChannelLenCopier = prometheus.NewGaugeFunc(
Copy link
Contributor

Choose a reason for hiding this comment

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

I failed to understand why can't we just have one gauge metric IngestorChannelLen (defined in ingest.go) and use it for both batcher and copier with different label values? That approach would make it for less code and would not require to have a global var samplesCopierChannelToMonitor

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

The recent commit updates this. But basically, one metric (copier channel_len) needs a callback to be updated when prometheus pings the /metric (hence we need NewGaugeFunc), and the other done for metric batcher will be used irrespective of prometheus calls (so NewGauge will work).

@harkishen harkishen force-pushed the fix_promscale_refactoring_2 branch from 440b5a7 to 11dcae3 Compare February 21, 2022 11:18
Signed-off-by: Harkishen-Singh <harkishensingh@hotmail.com>
@harkishen harkishen force-pushed the fix_promscale_refactoring_2 branch from 8715201 to 07a5f03 Compare February 21, 2022 13:09
@harkishen harkishen enabled auto-merge (rebase) February 21, 2022 13:18
@harkishen harkishen merged commit 8e871f0 into timescale:master Feb 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants