cloud-init generator script execute very slow in dash

Bug #2030729 reported by wenao
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
cloud-init (Ubuntu)
Triaged
Medium
Unassigned

Bug Description

hello
when system start, or sytemd reload, the cloud-init generator script (https://github.com/canonical/cloud-init/blob/main/systemd/cloud-init-generator.tmpl) will execute, and this script will call ds-identify scirpt (https://github.com/canonical/cloud-init/blob/main/tools/ds-identify) in function check_for_datasource, and there is a fuction _read_config in ds-identify scirpt, this function (read file by `while read line`) will read all configrue content in cfg file. we found _read_config execute very slow in dash, bu fast in bash

this is a simple test scirpt
```
root@debtest:~# cat /root/test.sh
while read line;do
  echo $line
done < /root/test.txt
root@debtest:~# cat /root/test.txt
111
222
333

root@debtest:~# strace bash test.sh
...
read(0, "111\n222\n333\n", 4096) = 12
...

root@debtest:~#strace dash test.sh

read(0, "1", 1) = 1
read(0, "1", 1) = 1
read(0, "1", 1) = 1
read(0, "\n", 1) = 1
write(1, "111\n", 4111
) = 4
read(0, "2", 1) = 1
read(0, "2", 1) = 1
read(0, "2", 1) = 1
read(0, "\n", 1) = 1
write(1, "222\n", 4222
) = 4
read(0, "3", 1) = 1
read(0, "3", 1) = 1
read(0, "3", 1) = 1
read(0, "\n", 1) = 1
write(1, "333\n", 4333

```

in bash, read size is 4096, but in dash, read size is 1, so the dash need ready many many times if the configure file content is much, and we found in nfs/qemu_fw_cfg file system, (some cloud instance pass some cfg by qemu_fw_cfg file provided by kvm) , the cost time will be very long in dash

so we think it's better to modify the script interpreter to "/bin/bash" from "/bin/sh", if it is ok, we can submit a PR to change it.

Thanks

Revision history for this message
Scott Moser (smoser) wrote :

I feel like the right thing to do is either:
a. write a tiny yaml parser in c and use it
b. let 'grep' do much of the filtering.

'a' is a lot of work and pain to get integrated but could ultimately lead to a much higher payout of having ds-identify in c (or rust... some better language than shell).

I feel like 'b' is probably the right solution here even though it requires some re-work and shell forks to get there. dash was chosen by ubuntu as /bin/sh for good reason. dash *does* outperform bash in many/most cases its really unfortunate that this is so bad. (https://www.baeldung.com/linux/dash-vs-bash-performance)

What I suggest is
1. file a bug with dash with an example of how bad the 'read(1)' performs on any non-tiny file.
2. re-work check_config to use 'grep' for most of the filtering.

I've put up a PR at https://github.com/canonical/cloud-init/pull/4327 that shows '2'.

Revision history for this message
wenao (wenaojinkang) wrote :

Thank you for your reply and PR helped me solved this confusion

James Falcon (falcojr)
Changed in cloud-init (Ubuntu):
status: New → Triaged
importance: Undecided → Medium
Revision history for this message
Dong Liang (qidong-ld) wrote :

in the future version,systemd merged the patch :https://github.com/systemd/systemd/commit/5c1d67af465ab6921beec3f864ffdf1670ca4e1e ,which make qemu_fw_cfg be modprobed earlier than sytemd-generatior. If we use fw_cfg device to config cloud-init, cloud-init generation will use more than 90s to executing ds-identify. When the "cloud-init generator" runs for more than 90 seconds, systemd will kill it, and all cloud-init services will not be able to execute. This issue already exists in the Debian 12 operating system.

Revision history for this message
Scott Moser (smoser) wrote :

@Dong,

Feel free to open another issue with the content from #3. I can't think of an obvious correlation between that patch and ds-identify taking longer.

In doing so, please attach 'cloud-init collect-logs' after it has run.

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package cloud-init - 23.4~3g0cb0b80f-0ubuntu1

---------------
cloud-init (23.4~3g0cb0b80f-0ubuntu1) noble; urgency=medium

  * d/p/do-not-block-user-login.patch:
    - Revert behavior, allow user login after cloud-init stage (LP: #2039505)
  * d/control: add python3-apt as Recommends to read APT config from apt_pkg
  * drop the following cherry-picks now included:
    + d/p/cpick-0d9f149a-Pytestify-apt-config-test-modules-4424
    + d/p/cpick-5023e9f9-Refactor-test_apt_source_v1.py-to-use-pytest-4427
    + d/p/cpick-e9cdd7e3-Install-gnupg-if-gpg-not-found-4431
    + d/p/cpick-015543d3-apt-install-software-properties-common-when-absent-but
  * Upstream snapshot based on upstream/main at 0cb0b80f.
    - Bugs fixed in this snapshot: (LP: #2034273, #2030729)
      (LP: #2038945, #2039453)

 -- Chad Smith <email address hidden> Wed, 08 Nov 2023 22:24:09 -0700

Changed in cloud-init (Ubuntu):
status: Triaged → Fix Released
Revision history for this message
Alberto Contreras (aciba) wrote :
Changed in cloud-init (Ubuntu):
status: Fix Released → Triaged
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.