Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dist: The way to fix 'ordering cycle' issue on var-lib-scylla.mount might be wrong #8761

Closed
syuu1228 opened this issue May 31, 2021 · 5 comments
Assignees
Milestone

Comments

@syuu1228
Copy link
Contributor

We had an issue on Ubuntu 20.04 AMI that /var/lib/scylla does mounted after reboot (#8482), and it was because systemd causes 'ordering cycle' error on our systemd units (var-lib-scylla.mount and var-lib-systemd-coredump.mount):

scyllaadm@ip-172-31-43-110:~$ sudo journalctl -xe|grep local-fs.target
Apr 15 19:57:59 ip-172-31-43-110 systemd[1]: local-fs.target: Found ordering cycle on var-lib-systemd-coredump.mount/start
Apr 15 19:57:59 ip-172-31-43-110 systemd[1]: local-fs.target: Found dependency on var-lib-scylla.mount/start
Apr 15 19:57:59 ip-172-31-43-110 systemd[1]: local-fs.target: Found dependency on local-fs.target/start
Apr 15 19:57:59 ip-172-31-43-110 systemd[1]: local-fs.target: Job var-lib-systemd-coredump.mount/start deleted to break ordering cycle starting with local-fs.target/start
Apr 15 19:57:59 ip-172-31-43-110 systemd[1]: local-fs.target: Found ordering cycle on var-lib-scylla.mount/start
Apr 15 19:57:59 ip-172-31-43-110 systemd[1]: local-fs.target: Found dependency on local-fs.target/start
Apr 15 19:57:59 ip-172-31-43-110 systemd[1]: local-fs.target: Job var-lib-scylla.mount/start deleted to break ordering cycle starting with local-fs.target/start
-- Subject: A start job for unit local-fs.target has finished successfully
-- A start job for unit local-fs.target has finished successfully.

And we fixed it by adding DefaultDependencies=no on these mount units at
d92a266.

However, I realized that auto-generated mount units on Ubuntu 20.04 specifies Before=local-fs.target:

vagrant@vagrant:/run/systemd/generator$ cat ./boot-efi.mount 
# Automatically generated by systemd-fstab-generator

[Unit]
Documentation=man:fstab(5) man:systemd-fstab-generator(8)
SourcePath=/etc/fstab
Before=local-fs.target
Requires=systemd-fsck@dev-disk-by\x2duuid-8154\x2d44B6.service
After=systemd-fsck@dev-disk-by\x2duuid-8154\x2d44B6.service
After=blockdev@dev-disk-by\x2duuid-8154\x2d44B6.target

[Mount]
Where=/boot/efi
What=/dev/disk/by-uuid/8154-44B6
Type=vfat
Options=umask=0077
vagrant@vagrant:/run/systemd/generator$ cat ./-.mount 
# Automatically generated by systemd-fstab-generator

[Unit]
Documentation=man:fstab(5) man:systemd-fstab-generator(8)
SourcePath=/etc/fstab
Before=local-fs.target
After=blockdev@dev-mapper-vgvagrant\x2droot.target

[Mount]
Where=/
What=/dev/mapper/vgvagrant-root
Type=ext4
Options=errors=remount-ro

It is opposite on our mount units, we specifies After=local-fs.target:
https://github.com/scylladb/scylla/blob/master/dist/common/scripts/scylla_raid_setup#L164

I suspect this is causing ordering cycle, so I changed the mount unit to Before=local-fs.target scylla-server.service and dropped DefaultDependencies=no, tested if it works after reboot, then I found it works:

scyllaadm@ip-172-31-4-22:~$ cat /etc/systemd/system/var-lib-scylla.mount 
[Unit]
Description=Scylla data directory
Before=local-fs.target scylla-server.service
After=mdmonitor.service

[Mount]
What=/dev/disk/by-uuid/582d2d8b-f4b7-478b-97b6-b86d18eb000c
Where=/var/lib/scylla
Type=xfs
Options=noatime

[Install]
WantedBy=multi-user.target

So, I think we fixed the issue wrong way, we just needed to move local-fs.target from After= to Before=.

syuu1228 added a commit to syuu1228/scylla that referenced this issue May 31, 2021
All mount units generated by systemd-fstab-generator have
"Before=local-fs.target", it is opposite of our mount units.
Seems like it is the reason we got "ordering cycle" error on scylladb#8482,
we need to move local-fs.target to Before= to fix the error.

Fixes scylladb#8761
@slivne slivne added this to the 4.7 milestone Jun 15, 2021
@slivne slivne modified the milestones: 5.0, 5.2 Apr 14, 2022
@DoronArazii DoronArazii modified the milestones: 5.2, 5.x Nov 22, 2022
@yaronkaikov
Copy link
Contributor

@syuu1228 do we have this issue in our current images? if not let's close this

syuu1228 added a commit to syuu1228/scylla that referenced this issue Oct 5, 2023
systemd man page says:

systemd-fstab-generator(3) automatically adds dependencies of type Before= to
all mount units that refer to local mount points for this target unit.

So Before=local-fs.taget is the correct dependency for local mount
points, but we currently specify "After=local-fs.target", it should be
fixed.

Fixes scylladb#8761
syuu1228 added a commit to syuu1228/scylla that referenced this issue Oct 5, 2023
systemd man page says:

systemd-fstab-generator(3) automatically adds dependencies of type Before= to
all mount units that refer to local mount points for this target unit.

So "Before=local-fs.taget" is the correct dependency for local mount
points, but we currently specify "After=local-fs.target", it should be
fixed.

Fixes scylladb#8761
@syuu1228
Copy link
Contributor Author

syuu1228 commented Oct 5, 2023

@yaronkaikov I realized the issue is still not fixed, so I send a PR for this: #15647

syuu1228 added a commit to syuu1228/scylla that referenced this issue Nov 6, 2023
systemd man page says:

systemd-fstab-generator(3) automatically adds dependencies of type Before= to
all mount units that refer to local mount points for this target unit.

So "Before=local-fs.taget" is the correct dependency for local mount
points, but we currently specify "After=local-fs.target", it should be
fixed.

Also replaced "WantedBy=multi-user.target" with "WantedBy=local-fs.target",
since .mount are not related with multi-user but depends local
filesystems.

Fixes scylladb#8761
syuu1228 added a commit to syuu1228/scylla that referenced this issue Nov 6, 2023
systemd man page says:

systemd-fstab-generator(3) automatically adds dependencies of type Before= to
all mount units that refer to local mount points for this target unit.

So "Before=local-fs.taget" is the correct dependency for local mount
points, but we currently specify "After=local-fs.target", it should be
fixed.

Also replaced "WantedBy=multi-user.target" with "WantedBy=local-fs.target",
since .mount are not related with multi-user but depends local
filesystems.

Fixes scylladb#8761
denesb pushed a commit that referenced this issue Nov 6, 2023
systemd man page says:

systemd-fstab-generator(3) automatically adds dependencies of type Before= to
all mount units that refer to local mount points for this target unit.

So "Before=local-fs.taget" is the correct dependency for local mount
points, but we currently specify "After=local-fs.target", it should be
fixed.

Also replaced "WantedBy=multi-user.target" with "WantedBy=local-fs.target",
since .mount are not related with multi-user but depends local
filesystems.

Fixes #8761

Closes #15647
@denesb
Copy link
Contributor

denesb commented Dec 18, 2023

@syuu1228 should we backport this?

@syuu1228
Copy link
Contributor Author

@denesb Yes

denesb pushed a commit that referenced this issue Dec 19, 2023
systemd man page says:

systemd-fstab-generator(3) automatically adds dependencies of type Before= to
all mount units that refer to local mount points for this target unit.

So "Before=local-fs.taget" is the correct dependency for local mount
points, but we currently specify "After=local-fs.target", it should be
fixed.

Also replaced "WantedBy=multi-user.target" with "WantedBy=local-fs.target",
since .mount are not related with multi-user but depends local
filesystems.

Fixes #8761

Closes #15647

(cherry picked from commit a232783)
denesb pushed a commit that referenced this issue Dec 19, 2023
systemd man page says:

systemd-fstab-generator(3) automatically adds dependencies of type Before= to
all mount units that refer to local mount points for this target unit.

So "Before=local-fs.taget" is the correct dependency for local mount
points, but we currently specify "After=local-fs.target", it should be
fixed.

Also replaced "WantedBy=multi-user.target" with "WantedBy=local-fs.target",
since .mount are not related with multi-user but depends local
filesystems.

Fixes #8761

Closes #15647

(cherry picked from commit a232783)
@denesb
Copy link
Contributor

denesb commented Dec 19, 2023

Backported to 5.4 and 5.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants