filestorageextension should allow for a maximum storage limit

[mattsains]

Component(s)

extension/storage/filestorage

Is your feature request related to a problem? Please describe.

See also: open-telemetry/opentelemetry-collector#12634

Although the file storage extension has fields to garbage collect data on disk, there is no way to set a limit of how much space the extension can use on disk. There are limits that can be set, for example, in the exporter helper in terms of number of items being stored, but not megabytes, and not in total for a single extension.

I think this is an important feature because filling up a disk can be an reliability and performance issue.

Describe the solution you'd like

The filestorage extension should allow for a new field to set the absolute maximum amount of storage used by the extension. If the extension reaches this maximum during a call to Set(), it should automatically compact if appropriate, and, if still at the limit, should return a storage contract-defined error to communicate that it is full and cannot accept new data.

Additionally, fields ReboundNeededThresholdMiB and ReboundTriggerThresholdMiB should be validated to be at or lower than this maximum size

Describe alternatives you've considered

As mentioned, there is exporterhelper limit on queue size, but pdata entries can vary wildly in size, and also a single storage extension can be used for multiple purposes.

Additional context

No response

[github-actions]

Pinging code owners:

• extension/storage/filestorage: @swiatekm @VihasMakwana

See Adding Labels via Comments if you do not have permissions to add labels yourself.

[swiatekm]

Sounds like a useful feature, and I don't see any immediate problems with the proposed implementation. If you want to define a standard error for this, then https://github.com/open-telemetry/opentelemetry-collector/blob/main/extension/xextension/storage/storage.go would be the place, but in practice it'd probably be easier to first submit a PR to the filestorage extension and get that reviewed.

@andrzej-stencel I recall discussing this in the past. Do you recall any potential problems with this feature?

[swiatekm]

/label -needs-triage

[VihasMakwana]

https://github.com/open-telemetry/opentelemetry-collector/blob/main/extension/xextension/storage/storage.go would be the place, but in practice it'd probably be easier to first submit a PR to the filestorage extension and get that reviewed.

I agree. It's a good starting point.

@mattsains will you be able to submit a PR for this?

[mattsains]

Absolutely I’ll get on it

[mattsains]

one thing I've noticed here is that the compaction process is constructive and creates a compacted copy of the database during the compaction process. It seems to me that either the maximum size configuration should not include the size taken up by compaction (which could end up temporarily consuming as much space as the original database), or by basically dividing the max size by two when compaction is enabled. Thoughts?

[VihasMakwana]

It seems to me that either the maximum size configuration should not include the size taken up by compaction (which could end up temporarily consuming as much space as the original database), or by basically dividing the max size by two when compaction is enabled

We should not should not include the size taken up by compaction. Compaction db creates a "temporary" drop and we delete it once compaction is done.

[mattsains]

@VihasMakwana I'm thinking of otel users setting a max storage size and then getting confused when we're using up potentially double of the storage they've allocated to otel. Very surprising and potentially kills the collector if the user has otel running on a volume with not much more size than the max

[VihasMakwana]

But that would be temporary, right?

Documenting this behaviour looks like a way forward. Thoughts?

Very surprising and potentially kills the collector if the user has otel running on a volume with not much more size than the max

I guess this could right now as well i.e. without the "maximum storage limit" option if compaction is enabled. On a side note, no new data will be written/read while compaction is taking place.

[mattsains]

yes, it would be temporary. But temporarily we would be violating the "don't exceed this storage size" constraint from the user. Maybe that's fine, I'm not sure