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

[Feature request] dropping chunk throttle #2384

Closed
mtanda opened this Issue Feb 1, 2017 · 9 comments

Comments

Projects
None yet
3 participants
@mtanda
Copy link
Contributor

mtanda commented Feb 1, 2017

Current implementation doesn't consider prometheus_local_storage_persistence_urgency_score.
https://github.com/prometheus/prometheus/blob/v1.5.0/storage/local/storage.go#L1210-L1250

I think it should consider prometheus_local_storage_persistence_urgency_score to prevent, dropping chunk activity cause too many disk iops, and finally cause rush mode.

@beorn7

This comment has been minimized.

Copy link
Member

beorn7 commented Feb 1, 2017

That's a good point. While maintenance of in-memory series needs to be fast in rushed mode to persist more chunks, obviously all archived chunks are persisted already.

@brian-brazil

This comment has been minimized.

Copy link
Member

brian-brazil commented Feb 1, 2017

With a server in effectively permanent rushed mode (which is going to be most large servers), if this is slowed down then you're going to run out of disk space.

These iops do not contend with the persistence iops, as at most one of these functions can be running at a given time.

@beorn7

This comment has been minimized.

Copy link
Member

beorn7 commented Feb 1, 2017

Right. I thought for a while the maintenance of archived series would be sped up as well with increased persist urgency. But I did the right thing from the beginning, see https://github.com/prometheus/prometheus/blob/v1.5.0/storage/local/storage.go#L1238

So yeah, I think leaving it in the normal even throttling is the right thing to do. In rushed mode, the maintenance frequency of archived series will be much lower than the one for memory series, so that's the right trade off.

@beorn7 beorn7 closed this Feb 1, 2017

@mtanda

This comment has been minimized.

Copy link
Contributor Author

mtanda commented Feb 2, 2017

@beorn7

Actually, we have a problem to drop too old chunks from our production Prometheus server.

We use Prometheus more than 1 year.
We set the retention period to 1 year (I know it is extraordinary use case) 1 year ago, and now the disk usage becomes too high, we need to reduce disk usage.
To reduce disk usage, we want to drop old chunks by setting retention period to 6 months.

But, it is too difficult to dropping chunks.
Prometheus need to read so much chunks and sometimes need to write so much chunks (partial drop cause copy chunks from file A to file B).
In our test, setting retention period to 11 months, it cause too much I/O, prometheus_local_storage_persistence_urgency_score becomes worse.

To accomplish our goal, I consider to optimize dropping chunk code.

  1. don't read chunk itself if the last time of time series is older than drop retention like following (not tested yet...)
    mtanda@ef5fa35#diff-4e4190468707536b578e5df823345c0eR883

  2. search drop-able position by binary search
    https://github.com/prometheus/prometheus/blob/v1.5.0/storage/local/persistence.go#L884-L920
    This part, to search drop-able position, do linear search.
    I think if all chunk is ordered by time, we can use binary search.

I'm not familiar with Prometheus code, so I might misunderstand something.
Please give me feedback.

@beorn7

This comment has been minimized.

Copy link
Member

beorn7 commented Feb 2, 2017

In general, the Prometheus server is not optimized for sudden decreases of the retention time (and I'm reluctant to do so if that increases the code complexity significantly or if it makes the regular use case (i.e. a constant or slightly changing retention time) worse).

About your suggestions:

  1. seems straight forward enough, but it will only make a dent for the sudden dramatic decrease of retention time you have performed in your use case. Since you already did the coding, how about creating a PR? I might be convinced to accept it, but it would also give others the chance to chime in. (Please document the rationale in doc comments.)

  2. The assumption is that in most cases, the cut off will happen very close to the beginning of the file, so a binary search will probably make things worse in the average case. Also, it will "seek backwards", while the current approach creates a forward read pattern (which might only matter on HDD). This would definitely need benchmarking. But a first step would be to establish if it is a bottleneck at all.

@mtanda

This comment has been minimized.

Copy link
Contributor Author

mtanda commented Feb 2, 2017

Of course, I don't want to add complexity or bad effect to Prometheus, too.

seems straight forward enough, but it will only make a dent for the sudden dramatic decrease of retention time you have performed in your use case. Since you already did the coding, how about creating a PR? I might be convinced to accept it, but it would also give others the chance to chime in. (Please document the rationale in doc comments.)

I'll try to make a PR for this.
And I think it may be worth if the environment is highly dynamic, the Prometheus store many short (the lifetime is 1 day or less) time series.
In such environment, reduce retention period only 1 day cause much I/O.

The assumption is that in most cases, the cut off will happen very close to the beginning of the file, so a binary search will probably make things worse in the average case. Also, it will "seek backwards", while the current approach creates a forward read pattern (which might only matter on HDD). This would definitely need benchmarking. But a first step would be to establish if it is a bottleneck at all.

I'm sorry, I miss the point. And I understand backwards read (and random read?) is not good for HDD.
The binary search is a bad approach for most of the cases.

I reconsider how I can drop too old chunks effectively.
Adding time range options to "deleting series" API might be good.
https://prometheus.io/docs/querying/api/#deleting-series

In "deleting series" API, we can take more specific optimization method.
I'll make another feature request for this feature.

@mtanda

This comment has been minimized.

Copy link
Contributor Author

mtanda commented Feb 2, 2017

I found the same issue what I met.
I'm not only one doing such strange thing :-)
#1740

@beorn7

This comment has been minimized.

Copy link
Member

beorn7 commented Feb 2, 2017

#1740 has been commented on already. Obviously, truncating series files at the right place requires IO. If you reduce the retention time, the work has to be done. If your server was heavily loaded already, it might need to throttle ingestion to do so.

@lock

This comment has been minimized.

Copy link

lock bot commented Mar 24, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked and limited conversation to collaborators Mar 24, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.