Implement new automatic snapshot feature #2403

ip1981 · 2020-11-26T13:27:20Z

Signed-off-by: Igor Pashev [email protected]

This change is

ip1981 · 2020-11-26T13:33:50Z

iml-wire-types/src/snapshot.rs

+#[cfg_attr(feature = "graphql", derive(juniper::GraphQLObject))]
+#[derive(serde::Deserialize, serde::Serialize, Eq, Clone, Debug)]
+/// Automatic snapshot policy
+pub struct SnapshotPolicy {


FYI, currently weeks end on Sunday. I think it would be the best to make it part of the policy, even if not visible for users.

iml-api/src/graphql/mod.rs

iml-gui/crate/src/page/snapshot/create_policy.rs

johnsonw · 2020-12-02T19:18:29Z

iml-services/iml-snapshot/src/policy.rs

+            svcs.insert(p, ah);
+        }
+
+        tokio::time::delay_for(tokio::time::Duration::from_secs(6)).await;


Any significance to 6 seconds?

johnsonw · 2020-12-02T20:03:10Z

iml-services/iml-snapshot/src/policy.rs

+    start: NaiveDateTime,
+    datetimes: &[NaiveDateTime],
+) -> LinkedList<LinkedList<NaiveDateTime>> {
+    let mut v: Vec<i64> = datetimes


Any benefit to using a linked list? If we could use a Set we wouldn't need to worry about de-duping.

It's just that the size is not known in advance. And more importantly, the order is important.

iml-wire-types/src/snapshot.rs

iml-wire-types/src/warp_drive.rs

nixuser · 2020-12-03T16:28:16Z

Web interface shows Failed to load resource: the server responded with a status of 502 () error, when I applied this build.

nixuser · 2020-12-03T16:34:15Z

why do we have policy and interval in help output? is it overlapping functionality or it is different?
SUBCOMMANDS:
create Create a snapshot
destroy Destroy the snapshot
help Prints this message or the help of the given subcommand(s)
interval Snapshot intervals operations
list List snapshots
mount Mount a snapshot
policy Automatic snapshot policies operations
retention Snapshot retention rules operations
unmount Unmount a snapshot

ip1981 · 2020-12-04T10:40:12Z

why do we have policy and interval in help output? is it overlapping functionality or it is different?
SUBCOMMANDS:
create Create a snapshot
destroy Destroy the snapshot
help Prints this message or the help of the given subcommand(s)
interval Snapshot intervals operations
list List snapshots
mount Mount a snapshot
policy Automatic snapshot policies operations
retention Snapshot retention rules operations
unmount Unmount a snapshot

It's a previous effort which will be replaced by this new feature. You can ignore it for now.

ip1981 · 2020-12-04T10:43:24Z

Web interface shows Failed to load resource: the server responded with a status of 502 () error, when I applied this build.

It's probably due to database migrations. I guess it needs to be built from scratch after I resolve conflicts in this PR.

iml-gui/crate/src/page/snapshot/create_policy.rs

jgrund · 2020-12-11T16:49:03Z

@ip1981 please rebase, this has conflicts on latest master.

iml-api/src/graphql/mod.rs

jgrund · 2020-12-11T17:11:57Z

iml-api/src/graphql/mod.rs

+        ),
+    ))]
+    /// Creates a new automatic snapshot policy.
+    async fn create_snapshot_policy(


Let's move this (and any other snapshot related queries / mutations) into a new snapshot module so it's consistent with other areas.

Is it possible? Looks like we put everything into the implementation of pub(crate) struct QueryRoot.

Yup, we are doing this in a few other areas already.

ref:

integrated-manager-for-lustre/iml-api/src/graphql/stratagem.rs

Line 24 in 671b0be

pub(crate) struct StratagemQuery;

jgrund · 2020-12-11T17:16:36Z

iml-api/src/graphql/mod.rs

+        sqlx::query!(
+            r#"
+                DELETE FROM snapshot_policy
+                WHERE (filesystem IS NOT DISTINCT FROM $1)


Why not !=

Because $1 can be NULL.

Given we can only have one policy per fs, why do we need the id at all?

Why not just make filesystem the only required param as it's already a unique key?

This was written first when I thought id would be used in general and fs only for convenience. I will update.

iml-api/src/graphql/mod.rs

jgrund · 2020-12-11T17:17:42Z

iml-api/src/graphql/mod.rs

+            r#"
+                DELETE FROM snapshot_policy
+                WHERE (filesystem IS NOT DISTINCT FROM $1)
+                OR    (id IS NOT DISTINCT FROM $2)


Likewise, $2 can be NULL. I don't actually use id. So here it's just a PoC.

Yeah, I think we should eliminate the id and just make the fs field required.

... and it will be = ;-)

iml-api/src/graphql/mod.rs

ip1981 · 2020-12-23T13:52:47Z

Some more things to implement:
* Min snapshots are 2. Max snapshots are 32.

Done at the DB level and in GUI.

* We need to schedule the snapshot cleanup window per-policy
* Each interval scheduled must have the ability to specify when it will first run

Done. First create a new snapshot if possible, then cleanup.

There are a couple of things to improve, but in general it's ready.

~~I might be hitting a bug in Chrome when an input event is not triggered when selecting a date.~~
Was with 81, ok with 87.

Signed-off-by: Igor Pashev <[email protected]>

ip1981 · 2021-02-25T11:50:51Z

I will need #2457 and #2458

Signed-off-by: Igor Pashev <[email protected]>

ip1981 added the enhancement label Nov 26, 2020

ip1981 requested a review from a team November 26, 2020 13:27

ip1981 self-assigned this Nov 26, 2020

ip1981 force-pushed the ip1981/EX-1835-snapshot-v2-svc branch from 93fa574 to 3b789d1 Compare November 26, 2020 13:29

ip1981 commented Nov 26, 2020

View reviewed changes

ip1981 force-pushed the ip1981/EX-1835-snapshot-v2-svc branch 3 times, most recently from 14e32b4 to 5d15364 Compare November 30, 2020 15:20

johnsonw reviewed Dec 3, 2020

View reviewed changes

ip1981 force-pushed the ip1981/EX-1835-snapshot-v2-svc branch 7 times, most recently from e3a0efd to 2cff950 Compare December 8, 2020 17:36

ip1981 commented Dec 9, 2020

View reviewed changes

iml-gui/crate/src/page/snapshot/create_policy.rs Outdated Show resolved Hide resolved

jgrund force-pushed the ip1981/EX-1835-snapshot-v2-svc branch from 2cff950 to 17df2d3 Compare December 10, 2020 02:24

jgrund requested review from johnsonw and jgrund December 11, 2020 16:59

jgrund suggested changes Dec 11, 2020

View reviewed changes

jgrund reviewed Dec 11, 2020

View reviewed changes

iml-api/src/graphql/mod.rs Outdated Show resolved Hide resolved

jgrund reviewed Dec 11, 2020

View reviewed changes

iml-api/src/graphql/mod.rs Outdated Show resolved Hide resolved

ip1981 marked this pull request as ready for review December 23, 2020 13:03

ip1981 force-pushed the ip1981/EX-1835-snapshot-v2-svc branch 2 times, most recently from eac5624 to 3d4c89c Compare December 23, 2020 13:49

ip1981 force-pushed the ip1981/EX-1835-snapshot-v2-svc branch from 3d4c89c to c59d2bd Compare December 23, 2020 14:11

ip1981 marked this pull request as draft December 23, 2020 17:54

ip1981 force-pushed the ip1981/EX-1835-snapshot-v2-svc branch 3 times, most recently from a128baa to ec17b4f Compare December 24, 2020 14:17

ip1981 marked this pull request as ready for review December 24, 2020 14:31

ip1981 force-pushed the ip1981/EX-1835-snapshot-v2-svc branch from ec17b4f to b4c5354 Compare December 24, 2020 19:59

ip1981 marked this pull request as draft February 15, 2021 16:03

ip1981 changed the base branch from master to b6.3 February 15, 2021 16:03

Disable building debug packages

acdbb38

Signed-off-by: Igor Pashev <[email protected]>

ip1981 force-pushed the ip1981/EX-1835-snapshot-v2-svc branch from b4c5354 to 4298bef Compare February 16, 2021 12:34

ip1981 added 2 commits February 16, 2021 18:37

Do not install yarn if available

06e39df

Signed-off-by: Igor Pashev <[email protected]>

Require cryptography which works with Python2

58a5790

Signed-off-by: Igor Pashev <[email protected]>

ip1981 force-pushed the ip1981/EX-1835-snapshot-v2-svc branch 3 times, most recently from c6fd0fe to 036af3e Compare February 17, 2021 17:49

Avoid making a huge intermediate tarball

4d3d95a

Signed-off-by: Igor Pashev <[email protected]>

ip1981 force-pushed the ip1981/EX-1835-snapshot-v2-svc branch 4 times, most recently from 4a1bbdc to 34c0016 Compare February 25, 2021 11:36

ip1981 force-pushed the ip1981/EX-1835-snapshot-v2-svc branch 2 times, most recently from 76c9662 to 002be2a Compare February 27, 2021 15:07

Implement new automatic snapshot feature

d3d62fd

Signed-off-by: Igor Pashev <[email protected]>

ip1981 force-pushed the ip1981/EX-1835-snapshot-v2-svc branch from 002be2a to d3d62fd Compare February 27, 2021 17:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement new automatic snapshot feature #2403

Implement new automatic snapshot feature #2403

ip1981 commented Nov 26, 2020 •

edited

Loading

ip1981 Nov 26, 2020 •

edited

Loading

johnsonw Dec 2, 2020

ip1981 Dec 4, 2020

johnsonw Dec 2, 2020

ip1981 Dec 4, 2020

nixuser commented Dec 3, 2020

nixuser commented Dec 3, 2020

ip1981 commented Dec 4, 2020

ip1981 commented Dec 4, 2020

jgrund commented Dec 11, 2020

jgrund Dec 11, 2020

ip1981 Dec 11, 2020

jgrund Dec 11, 2020

jgrund Dec 11, 2020

ip1981 Dec 11, 2020 •

edited

Loading

jgrund Dec 11, 2020

ip1981 Dec 11, 2020

jgrund Dec 11, 2020

ip1981 Dec 11, 2020

jgrund Dec 11, 2020

ip1981 Dec 11, 2020

ip1981 commented Dec 23, 2020 •

edited

Loading

ip1981 commented Feb 25, 2021

Implement new automatic snapshot feature #2403

Are you sure you want to change the base?

Implement new automatic snapshot feature #2403

Conversation

ip1981 commented Nov 26, 2020 • edited Loading

ip1981 Nov 26, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nixuser commented Dec 3, 2020

nixuser commented Dec 3, 2020

ip1981 commented Dec 4, 2020

ip1981 commented Dec 4, 2020

jgrund commented Dec 11, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ip1981 Dec 11, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ip1981 commented Dec 23, 2020 • edited Loading

ip1981 commented Feb 25, 2021

ip1981 commented Nov 26, 2020 •

edited

Loading

ip1981 Nov 26, 2020 •

edited

Loading

ip1981 Dec 11, 2020 •

edited

Loading

ip1981 commented Dec 23, 2020 •

edited

Loading