Skip to content

feat: Add target ping interval#948

Merged
burningalchemist merged 3 commits intoburningalchemist:masterfrom
destinyoooo:fix_interval
Mar 27, 2026
Merged

feat: Add target ping interval#948
burningalchemist merged 3 commits intoburningalchemist:masterfrom
destinyoooo:fix_interval

Conversation

@destinyoooo
Copy link
Contributor

This PR includes the following improvements:

  1. Add target ping interval :
  • Added pingInterval field in target.go, defaulting to 30 seconds
  • Implemented ping caching mechanism, only ping the database when the last ping was more than pingInterval ago
  • Reduced unnecessary ping operations to the database, especially when using cached collectors

@burningalchemist
Copy link
Owner

Hey @destinyoooo, I have a small question here:

30 seconds is a hardcoded parameter. Do you think there are scenarios where it needs to be adjusted by the user?

Currently, one of the options to reduce that constant pinging (which isn't good for data warehouse solutions cost-wise) is to use the flag to disable ping completely (at a cost of potential connection losses).

So I'm wondering whether we need to expose pingInterval in configuration, or it's a sane default to have. What do you think?

@destinyoooo
Copy link
Contributor Author

Hey @destinyoooo, I have a small question here:

30 seconds is a hardcoded parameter. Do you think there are scenarios where it needs to be adjusted by the user?

Currently, one of the options to reduce that constant pinging (which isn't good for data warehouse solutions cost-wise) is to use the flag to disable ping completely (at a cost of potential connection losses).

So I'm wondering whether we need to expose pingInterval in configuration, or it's a sane default to have. What do you think?

Hello! I've completed the modification to change pingInterval from a hardcoded 30 seconds to a configurable parameter. Here are the related improvements:

  1. Added configuration option : Added a ping_interval parameter in the global configuration section, allowing users to adjust it as needed.

  2. Default value setting : Set the default value to 0, which means ping every time, maintaining the same behavior as before.

  3. Flexible configuration :

    • Set to 0: Ping every time during scrape
    • Set to a value greater than 0: Ping at the specified interval, e.g., 30s means ping every 30 seconds

@destinyoooo
Copy link
Contributor Author

In my opinion. This improvement is very necessary because:

  • For services like data warehouses that charge per query, reducing unnecessary ping operations can lower costs
  • Database performance and stability vary across different environments, and users may need to adjust the ping interval based on actual conditions
  • It provides a middle ground between completely disabling ping and pinging every time

@burningalchemist
Copy link
Owner

Yeah, makes sense to me. 👍 Let me have a brief look and we merge it soon. 👍

Copy link
Owner

@burningalchemist burningalchemist left a comment

Choose a reason for hiding this comment

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

There is a minor nitpick on the comment, otherwise looks good to me 👍

@burningalchemist burningalchemist changed the title fix: Add target ping interval feat: Add target ping interval Mar 27, 2026
@burningalchemist burningalchemist merged commit 87e6494 into burningalchemist:master Mar 27, 2026
4 checks passed
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.

2 participants