leth
Repos
13
Followers
28
Following
14

Events

issue comment
Extremely high upload rate burst, despite rate limit

If you're up for building from source to help test a fix, see https://github.com/kopia/kopia/pull/2682 :)

Created at 2 days ago
delete branch
leth delete branch 34-support-for-extra-zeros-like-in-factory1920201
Created at 1 week ago
issue comment
Support for extra zeros like in ::factory("192.0.2.01" )

This validation filter is defined by filter_var($address, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4); we'd need to replace that function with an alternative. Once we're past the validation, it looks like ip2long will be okay with the data!

Created at 1 week ago
create branch
leth create branch 34-support-for-extra-zeros-like-in-factory1920201
Created at 1 week ago
issue comment
BTRFS stats missing

Yeah, though you may have to wait for the next node_exporter release too (unless you're building from source), I don't really know how frequent they are!

Created at 2 weeks ago
pull request opened
Reduce privileges needed for btrfs device stats

Previously node_exporter needed to be running as a highly privileged user in order to get device stats.

The change which fixes this: https://github.com/dennwc/btrfs/pull/5 Improves #2193 Closes #2632

Created at 2 weeks ago

Reduce priviliges needed for btrfs device stats

Signed-off-by: Marcus Cobden leth@users.noreply.github.com

Created at 2 weeks ago
create branch
leth create branch btrfs-without-root
Created at 2 weeks ago
delete branch
leth delete branch run-as-non-root
Created at 2 weeks ago
push

Allow non-root users to query btrfs volumes

Created at 2 weeks ago
pull request opened
Reduce priviliges required to query btrfs volumes

In https://github.com/prometheus/node_exporter/issues/2632 we figured out that O_NOATIME was increasing the privileges needed!

From the open man page

O_NOATIME (since Linux 2.6.8)
              Do not update the file last access time (st_atime in the
              inode) when the file is [read(2)](https://man7.org/linux/man-pages/man2/read.2.html).

              This flag can be employed only if one of the following
              conditions is true:

              *  The effective UID of the process matches the owner UID
                 of the file.

              *  The calling process has the CAP_FOWNER capability in
                 its user namespace and the owner UID of the file has a
                 mapping in the namespace.
Created at 2 weeks ago
push

Allow non-root users to query btrfs volumes

Created at 2 weeks ago
create branch
leth create branch run-as-non-root
Created at 2 weeks ago
issue comment
BTRFS stats missing

It works, thank you so much for the help! I'll raise the PR on https://github.com/dennwc/btrfs to get this updated :)

Created at 2 weeks ago
issue comment
BTRFS stats missing

Oh nice! Thanks, I'll try and give that a try over the next few days.

On 11 Mar 2023, 21:56, at 21:56, Perry Naseck @.***> wrote:

Further info:

O_NOATIME (since Linux 2.6.8)
      Do not update the file last access time (st_atime in the
inode) when the file is
[read(2)](https://man7.org/linux/man-pages/man2/read.2.html).

      This flag can be employed only if one of the following
       conditions is true:

      *  The effective UID of the process matches the owner UID
         of the file.

      *  The calling process has the CAP_FOWNER capability in
         its user namespace and the owner UID of the file has a
         mapping in the namespace.

https://man7.org/linux/man-pages/man2/openat.2.html

So falling back to without O_NOATIME seems to be the best option.

-- Reply to this email directly or view it on GitHub: https://github.com/prometheus/node_exporter/issues/2632#issuecomment-1465031188 You are receiving this because you were mentioned.

Message ID: @.***>

Created at 2 weeks ago
issue comment
BTRFS stats missing

OpenFile maps to the open syscall. I just had a small look at the man page, perhaps the O_PATH flag might reduce the permissions required?

No luck unfortunately! The ioctl calls simply fail with "bad file descriptor".

Created at 2 weeks ago
issue comment
BTRFS stats missing

The error you're seeing comes from here: https://github.com/prometheus/node_exporter/blob/1724b28d27ff99fda3143207d1b0a52fa3a70122/collector/btrfs_linux.go#L128-L137

The error is returned from call here: https://github.com/dennwc/btrfs/blob/3097362dc07261a95e05ab5bd2494c343c9c0f38/btrfs.go#L31

OpenFile maps to the open syscall. I just had a small look at the man page, perhaps the O_PATH flag might reduce the permissions required?

I'll see if I can try it out

Created at 2 weeks ago
issue comment
BTRFS stats missing

The implementation here makes the same syscalls as btrfs device stats, so it'll need the same priviliges.

I was torn on what level to log at; the filesystem could be mounted in multple places, so it might not be a fatal error, and we fall back to the old metrics from /sys/fs anyway, and I didn't want to spam people's logs about a feature which they might find difficult to make work.

I never got around to joining the btrfs mailing list to feed back to them about the data in /sys/fs. It would be ideal if you could recreate btrfs device stats using it, but today you can't.

Created at 2 weeks ago
issue comment
BTRFS stats missing

The implementation gathers this data via syscalls rather than /sys/fs as there's not sufficient info in there to tell you which disk it is.

Can you try with debug logging enabled please! You should be able to grep it down to just things mentioning btrfs.

Created at 2 weeks ago
issue comment
feat(repository): Improve upload rate throttling

You can see the throttler being called during upload in this test: https://github.com/kopia/kopia/pull/2682/files#diff-43c9a863f13dd291b01f9db79a62110305a94196c8cb84e4c7d8e18a4f666b67R95-R111

It probably won't be a perfectly even throttler; as the buffer grows (see the test, I'm not sure why the buffer grows, perhaps it's go's built in copy implementation) it'll get a bit bursty. But since it's dealing with individual reads rather than whole uploads it should be a solid improvement.

If you want a really even upload rate I think you'd want to use a different throttler behind the upload-read-throttler. The current tokenbucket one makes sense for existing usages, 'saving up' for bandwidth kinda works well there as you want to make sure the # of concurrent uploads started fits within rates.

To ensure a really even read rate, you need to replenish tokens on an even schedule, and limit the amount of data read to what is available, instead of reading the full amount asked but making it take longer. I quick google found https://github.com/aybabtme/iocontrol which looks like a good implementation.

Created at 1 month ago
push

Add throttler test covering DuringUpload throttler

Created at 1 month ago
issue comment
feat(repository): Improve upload rate throttling

I've not tried yet. Do you have any docs on manual testing from source? I have kopia installed on windows so far.

Created at 1 month ago
push

Use internal iocopy for performance

Created at 1 month ago
started
Created at 1 month ago
push

Fix code linting issues

Created at 2 months ago
push

Fix code linting issues

Created at 2 months ago
issue comment
feat (repository): Improve upload rate throttling

I've fixed some of the linting issues. I'm not sure fixing the others is a good idea, given what the code is doing! I'd appreciate some guidance :)

Created at 2 months ago
push

Fix code linting issues

Created at 2 months ago
push

Rate limit data at start and during upload

This is to ensure both that we only start uploads we have bandwidth for, and that we don't exceed rate limits during uploads.

Created at 2 months ago
push

Improve upload rate throttling

Created at 2 months ago