Skip to content
This repository has been archived by the owner on Jul 25, 2022. It is now read-only.

Fix Stat deletion #2263

Merged
merged 3 commits into from
Sep 23, 2020
Merged

Fix Stat deletion #2263

merged 3 commits into from
Sep 23, 2020

Conversation

jgrund
Copy link
Member

@jgrund jgrund commented Sep 22, 2020

The current targets table is intermittently losing it's filesystems for
some targets.

This appears to be because we delete stat info before inserting the next
batch. If anyone queries during this gap, they will get empty data even
though we have the points, they just haven't been inserted yet.

In addition, we are clearing out all stats except a single point for any target colocated on the MGS node.

This patch waits until after insertion to do a deletion, and scopes the
delete so it should only effect the correct stat.

Fixes #2258.

Signed-off-by: Joe Grund jgrund@whamcloud.io


This change is Reviewable

@jgrund jgrund self-assigned this Sep 22, 2020
@jgrund jgrund requested a review from a team September 22, 2020 17:23
johnsonw
johnsonw previously approved these changes Sep 22, 2020
Implement an influx client extenstion that allows for direct
serialization to a struct instead of needing to traverse the result and
build it by hand. Ex:

```rust
struct MgsFsTime {
    time: u64,
}

let xs: Vec<MgsFsTime> = client
        .query_into(
            format!(
                r#"
            SELECT mgs_fs,is_mgs_fs
            FROM target
            WHERE mgs_fs='{}' AND is_mgs_fs=true
            ORDER BY time ASC
            LIMIT 2"#,
                fs_names
            )
            .as_str(),
            Some(Precision::Nanoseconds),
        )
        .await?
        .unwrap_or_default();
```

Signed-off-by: Joe Grund <jgrund@whamcloud.io>
The current targets table is intermittently losing it's filesystems for
some targets.

This appears to be because we delete stat info before inserting the next
batch. If anyone queries during this gap, they will get empty data even
though we have the points, they just haven't been inserted yet.

In addition, we are clearing out all stats except a single point for any target colocated on the MGS node.

This patch waits until after insertion to do a deletion, and scopes the
delete so it should only effect the correct stat.

Fixes #2258.

Signed-off-by: Joe Grund <jgrund@whamcloud.io>
Base automatically changed from influx-client-ext to master September 23, 2020 00:38
@jgrund jgrund dismissed johnsonw’s stale review September 23, 2020 00:38

The base branch was changed.

Copy link
Contributor

@mkpankov mkpankov left a comment

Choose a reason for hiding this comment

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

Reviewed 8 of 8 files at r1.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @jgrund)


iml-services/iml-stats/src/main.rs, line 496 at r1 (raw file):

        let entries: Vec<_> = xs
            .into_iter()
            .inspect(|x| {

I'd expect inspect to not mutate, maybe map?


iml-services/iml-stats/src/main.rs, line 551 at r1 (raw file):

            WHERE mgs_fs='{}' AND is_mgs_fs=true
            ORDER BY time ASC
            LIMIT 2"#,

Why LIMIT 2?

@jgrund
Copy link
Member Author

jgrund commented Sep 23, 2020


iml-services/iml-stats/src/main.rs, line 496 at r1 (raw file):

Previously, mkpankov (Michael Pankov) wrote…

I'd expect inspect to not mutate, maybe map?

it is not mutating the contained value, it's purely there for the side-effect of mutating fs_names, which is not associated with the combinator.
Using map would indicate the contained value is being mutated, which isn't the case here.

@jgrund
Copy link
Member Author

jgrund commented Sep 23, 2020


iml-services/iml-stats/src/main.rs, line 551 at r1 (raw file):

Previously, mkpankov (Michael Pankov) wrote…

Why LIMIT 2?

because if there is 1 or 0 it's a noop.

@jgrund jgrund requested review from a team and mkpankov September 23, 2020 13:01
@jgrund jgrund requested a review from a team September 23, 2020 13:31
Copy link
Contributor

@mkpankov mkpankov left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@jgrund jgrund merged commit de268f6 into master Sep 23, 2020
@jgrund jgrund deleted the fix-stat-deletion branch September 23, 2020 13:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants