grafana / loki

Like Prometheus, but for logs.
https://grafana.com/loki
GNU Affero General Public License v3.0
23.87k stars 3.45k forks source link

OrgID: allow no orgId to evenly distribute object name. #3363

Open sesky4 opened 3 years ago

sesky4 commented 3 years ago

Common S3 implementation can benefit from random key distribution. According to https://docs.aws.amazon.com/AmazonS3/latest/userguide/optimizing-performance.html

For example, your application can achieve at least 3,500 PUT/COPY/POST/DELETE or 5,500 GET/HEAD requests per second per prefix in a bucket. There are no limits to the number of prefixes in a bucket. You can increase your read or write performance by parallelizing reads. For example, if you create 10 prefixes in an Amazon S3 bucket to parallelize reads, you could scale your read performance to 55,000 read requests per second.

And in loki.yaml:

# Enables authentication through the X-Scope-OrgID header, which must be present # if true. If false, the OrgID will always be set to "fake". [auth_enabled: | default = true]

Currently when auth_enabled is set to false, all objects have a common prefix "fake/", which in my understanding will not benefit from that prefix distribution rule.

So I think removing "fake/" prefix when not running in multi-tenant mode can be helpful.

sesky4 commented 3 years ago

If I understant correctly, currently loki use orgId as object prefix to make hierarchy clean? If so, is it possible to remove that prefix? Since those object uuid key will not collide anyway.

sandstrom commented 3 years ago

Somewhat related, I'd also strongly suggest using a name/prefix other than fake. It's a bad name for many reasons (we didn't understand what it was, initially, and also risk that someone else will simply delete the directory thinking it's junk data).

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had any activity in the past 30 days. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

sandstrom commented 3 years ago

@sesky4 removing the prefix would be a good idea (because fake is a terribly confusing name), but it wouldn't increase S3 parallelism. For that to work, you'd need a set of multiple prefixes (higher number, more parallelism).

So a file like fake/9521c2b3b2eff1ca:178ac0e2fd3:178ac0e2ffd:3b20af23 would need to go into e.g. 95/9521c2b3b2eff1ca:178ac0e2fd3:178ac0e2ffd:3b20af23 (in this example scheme you'd take the first two characters of the file name and use as the prefix, that would give you 16^2 unique prefixes, and 256x parallelism).

All this said though, throughput in S3 is much better than it used to be, so these optimizations are less needed now. Not sure if this will make a practical difference.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had any activity in the past 30 days. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

sandstrom commented 3 years ago

@owen-d this bot is quite annoying, you're forcing me to write every N days to keep issues alive, despite them still being relevant.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had any activity in the past 30 days. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

sandstrom commented 3 years ago

@owen-d this is another example of your misconfigured bot 😄

owen-d commented 3 years ago

This is likely a problem in any mode of operation once a tenant reaches sufficient size. auth_enabled: false simply treats everything as a single tenant. I know we're going to do some storage layer refactoring and this will be a good candidate to address then.

cc @slim-bean @cyriltovena