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

tmpDir doesn't seem to be configurable #8498

Open
Chefaroon opened this issue Oct 27, 2024 · 14 comments
Open

tmpDir doesn't seem to be configurable #8498

Chefaroon opened this issue Oct 27, 2024 · 14 comments
Assignees

Comments

@Chefaroon
Copy link

I've been playing around with Dolt and I am annoyed at the constant creation of a ".tmp" directory in the same location as the binary file. I checked GitHub issues, I searched through the docs - didn't see anything related to a setting altering this. Furthermore I looked over the code (directly searching for ".tmp") and as far as I can see tmpDir doesn't seem to have any configurable ways - it is hardcoded to be always placed in the same directory as the binary.

If I am missing something, please let me know what I can add to my config file so I can place the tmpDir in appropriate place and not inside of /usr/local/bin, in which my dolt bin resides.

If I am not missing anything and tmpDir is indeed not configurable - please make it configurable! At the very least, temp data belongs to /tmp on GNU systems.

@timsehn
Copy link
Contributor

timsehn commented Oct 28, 2024

Thanks for the issue. I'm not too familiar with how we allocate .tmp. It would not surprise me if the Golang library we're using for temporary space creates a.tmp directory in the location of the binary.

We've shied away from OS specific implementations of temporary storage because they are fragile. But making it configurable may be an option.

@Chefaroon
Copy link
Author

I might be wrong but the directory appears to be created by reconfigIfTempFileMoveFails(), defined in dolt/go/cmd/dolt/system_checks.go:

func reconfigIfTempFileMoveFails(dEnv *env.DoltEnv) error {
	if !canMoveTempFile() {
		tmpDir := "./.dolt/tmp"

		if !dEnv.HasDoltDir() {
			tmpDir = "./.tmp"
		}

		stat, err := os.Stat(tmpDir)

		if err != nil {
			err := os.MkdirAll(tmpDir, os.ModePerm)

			if err != nil {
				return fmt.Errorf("failed to create temp dir '%s': %s", tmpDir, err.Error())
			}
		} else if !stat.IsDir() {
			return fmt.Errorf("attempting to use '%s' as a temp directory, but there exists a file with that name", tmpDir)
		}

		tempfiles.MovableTempFileProvider = tempfiles.NewTempFileProviderAt(tmpDir)
	}

	return nil
}

The logic looks for .dolt directory and places the .tmp in it when found. However, when the .dolt directory is configured to be somewhere else, not in the same path - the logic just dictates "make it here". At least that's how I understand this.

As for implementing OS specific temp storage logic - this is part of the os package and I don't think there's a valid reason not to use it:

https://pkg.go.dev/os#TempDir

@timsehn
Copy link
Contributor

timsehn commented Oct 28, 2024

We've had lots of tmp directory issues in the past (like 2019 era). It might be that the standard Go package has improved to the point where we can now use it.

@zachmu
Copy link
Member

zachmu commented Oct 28, 2024

The reason we use our own tmp directories is that Go's file system operations are very OS-specific, and certain operations break across partition boundaries on some OSes. To mitigate this problem, we drop tmp directories in locations we control, next to where the data files are stored.

An advanced user should be able to control this, though. We can give you a config setting to override it.

@macneale4 macneale4 self-assigned this Oct 28, 2024
@macneale4
Copy link
Contributor

Can you clarify what steps you are taking to get ".tmp" in /usr/local/bin? I'm not seeing that happen. What OS are you on? Are you running as a privileged user? Otherwise, I don't know how we'd have permission to even create that .tmp dir.

I'll add an option for you to specify the tmp dir of your choosing. Just want to be sure I understand how you are getting the behavior you are seeing.

Also, some operations will always use the ".dolt" directory for the purposes of building our data files. These operations are always the precursor to a storage update which needs to have everything in the same OS partition.

@macneale4
Copy link
Contributor

The seed for the temp directory is taken from the OS, and that will use the "TMPDIR" environment variable.

If you set TMPDIR, and does it help?

Are there specific dolt operations which are causing problems? Again, since I'm not seeing the behavior your are, I want to make I have as many repro steps as possible.

@macneale4
Copy link
Contributor

For more clarity, the only circumstance where we create the ".tmp" directory at all is when the OS prevents us from renaming a file in the OS's standard temp directory. So knowing the OS, and if you have a novel file system would be helpful.

@Chefaroon
Copy link
Author

Thanks for getting involved in this, Neil!

Can you clarify what steps you are taking to get ".tmp" in /usr/local/bin? I'm not seeing that happen. What OS are you on? Are you running as a privileged user? Otherwise, I don't know how we'd have permission to even create that .tmp dir.

When Dolt is installed system-wide (placed in /usr/local/bin/) and a standard OpenRC script is created for it so (it can run as a service), OpenRC changes its directory to the location of the binary (/usr/local/bin) and starts it from there. Since I am passing --config flag with data_dir defined in it (which puts .dolt dir somewhere else, not in $PWD) the temp folder gets created in /usr/local/bin/. I can recreate this by directly calling the dolt binary from anywhere in the file system and the temp folder always gets created in the current working directory.

For instance:

root@illi:/mnt# ll
total 8.0K
drwxr-xr-x  2 root root 4.0K Sep  5 17:25 .
drwxr-xr-x 24 root root 4.0K Oct 29 11:16 ..

root@illi:/mnt# dolt sql-server --config /opt/db.conf
Starting server with Config HP="127.0.0.1:3306"|T="5000"|R="false"|L="info"|S="/run/mysqld/mysqld.sock"
INFO[0000] Server ready. Accepting connections.
^CINFO[0002] Server closing listener. No longer accepting connections.

root@illi:/mnt# ll
total 12K
drwxr-xr-x  3 root root 4.0K Oct 29 11:26 .
drwxr-xr-x 24 root root 4.0K Oct 29 11:16 ..
drwxr-xr-x  2 root root 4.0K Oct 29 11:26 .tmp
root@illi:/mnt#

(*I have manually added the empty lines so it is easier to read)

The OpenRC script is just a boilerplate, only with the start-stop-daemon segment in start() changed to invoke dolt:

start() {
...
start-stop-daemon --start --background --make-pidfile --pidfile $pidfile --exec $cmd -- sql-server --config /opt/db.conf

The seed for the temp directory is taken from the OS, and that will use the "TMPDIR" environment variable.

If you set TMPDIR, and does it help?

Interestingly enough, the env var TMPDIR is not defined on the Alpine Linux instance I am using, which I know to be a standard env var in GNU systems. Setting the variable doesn't seem to make any difference though:

root@illi:/mnt# ll
total 12K
drwxr-xr-x  3 root root 4.0K Oct 29 11:26 .
drwxr-xr-x 24 root root 4.0K Oct 29 11:16 ..
drwxr-xr-x  2 root root 4.0K Oct 29 11:26 .tmp

root@illi:/mnt# rm -rf .tmp

root@illi:/mnt# export TMPDIR=/tmp

root@illi:/mnt# dolt sql-server --config /opt/db.conf
Starting server with Config HP="127.0.0.1:3306"|T="5000"|R="false"|L="info"|S="/run/mysqld/mysqld.sock"
INFO[0000] Server ready. Accepting connections.
^CINFO[0002] Server closing listener. No longer accepting connections.

root@illi:/mnt# ll
total 12K
drwxr-xr-x  3 root root 4.0K Oct 29 11:37 .
drwxr-xr-x 24 root root 4.0K Oct 29 11:16 ..
drwxr-xr-x  2 root root 4.0K Oct 29 11:37 .tmp

root@illi:/mnt# printenv | grep TMP
TMPDIR=/tmp
root@illi:/mnt#

Could it be the Go env var for temp (GOTMPDIR) rather than the OS one?

For clarity, I am using root user, the latest release of pre-compiled Dolt, obtained via your install.sh script, with the latest version of Alpine Linux (x64):

root@illi:/mnt# cat /etc/alpine-release
3.20.3

Since you can't reproduce this behaviour, and there doesn't seem to have been other reports of this in the past, this is likely a more isolated incident to how Alpine operates, in combination with the user trying to utilise the Dolt given settings to move things out of current working directory. Thus, a configuration setting for the temp data would quite likely fit best rather than considering changes on how the temp dir is appointed.

@macneale4
Copy link
Contributor

macneale4 commented Oct 29, 2024

Ok, I'm getting closer. running as root on a linux machine helps. log below. You can see we attempt to create the ./.tmp dir and get a permission error unless we run with root. I'm still not seeing it create the directory as root though - since the rename trigger is still not tripping.

Going to read through your steps and see if I missed anything, but by looking at the code it has to be that the rename which is failing:

err = file.Rename(name, testfile)

I'm going to see if I can trigger this by adding another file system.

ubuntu@ip-10-2-0-98:/tmp/blah$ cat /home/ubuntu/tmp_test/configuration/config.yml
log_level: debug

behavior:
  read_only: false
  autocommit: true

user:
  name: root
  password: ""

listener:
  host: localhost
  port: 3306
  max_connections: 100
  read_timeout_millis: 28800000
  write_timeout_millis: 28800000


data_dir: /home/ubuntu/tmp_test/data
ubuntu@ip-10-2-0-98:/tmp/blah$ dolt sql-server --config /home/ubuntu/tmp_test/configuration/config.yml
Starting server with Config HP="localhost:3306"|T="28800000"|R="false"|L="debug"|S="/tmp/mysql.sock"
open /home/ubuntu/tmp_test/data/.dolt/sql-server.info-4213836109: permission denied
ubuntu@ip-10-2-0-98:/tmp/blah$ sudo dolt sql-server --config /home/ubuntu/tmp_test/configuration/config.yml
Starting server with Config HP="localhost:3306"|T="28800000"|R="false"|L="debug"|S="/tmp/mysql.sock"
DEBU[0000] Loading events
INFO[0000] Server ready. Accepting connections.
WARN[0000] secure_file_priv is set to "", which is insecure.

@macneale4
Copy link
Contributor

The only reproduction I can successfully perform where we create the ".tmp" directory is when we have multiple file systems in play.

I don't see any mention of multiple mounted file systems in your description. Can you confirm one way or another? I suspect if you attempt to run dolt sql-server --config... in the same file system as where your data dir is you will stop seeing the .tmp artifact.

My repro is a little contrived, but I used a ramfs and I tweaked the dolt binary to log the error produced during the rename:

failed to rename /mnt/myramdisk/tmp/380767693 to ./testfile: rename /mnt/myramdisk/tmp/380767693 ./testfile: invalid cross-device link

Note, that I only was able to do this by setting TMPDIR env var, but the main call out is that our test of moving "./testfile" is effectively forcing you to start the binary on the same file system where you will store your data files. That is certainly a bug, and I'll get on fixing that.

@macneale4
Copy link
Contributor

macneale4 commented Oct 29, 2024

One more thing, I suspect your "/tmp" is on a different file system than /usr, which is what is causing all of this. This is why setting TMPDIR=/tmp has no effect.

@Chefaroon
Copy link
Author

One more thing, I suspect your "/tmp" is on a different file system than /usr, which is what is causing all of this. This is why setting TMPDIR=/tmp has no effect.

You are correct! Indeed, this does seem to relate, this test confirms your assumption:

root@illi:/mnt# ll
total 8.0K
drwxr-xr-x  2 root root 4.0K Oct 29 20:48 .
drwxr-xr-x 24 root root 4.0K Oct 29 11:16 ..

root@illi:/mnt# mkdir test_dolt

root@illi:/mnt# mkdir test_tmp

root@illi:/mnt# export TMPDIR=/mnt/test_tmp

root@illi:/mnt# echo $TMPDIR
/mnt/test_tmp

root@illi:/mnt# cd test_dolt/

root@illi:/mnt/test_dolt# /usr/local/bin/dolt sql-server
Starting server with Config HP="localhost:3306"|T="28800000"|R="false"|L="info"|S="/tmp/mysql.sock"
INFO[0000] Server ready. Accepting connections.
WARN[0000] secure_file_priv is set to "", which is insecure.
WARN[0000] Any user with GRANT FILE privileges will be able to read any file which the sql-server process can read.
WARN[0000] Please consider restarting the server with secure_file_priv set to a safe (or non-existent) directory.
^CINFO[0001] Server closing listener. No longer accepting connections.

root@illi:/mnt/test_dolt# ll
total 12K
drwxr-xr-x 3 root root 4.0K Oct 29 20:49 .
drwxr-xr-x 4 root root 4.0K Oct 29 20:49 ..
drwxr-xr-x 2 root root 4.0K Oct 29 20:49 .dolt

root@illi:/mnt/test_dolt# cd ..

root@illi:/mnt# ll
total 16K
drwxr-xr-x  4 root root 4.0K Oct 29 20:49 .
drwxr-xr-x 24 root root 4.0K Oct 29 11:16 ..
drwxr-xr-x  3 root root 4.0K Oct 29 20:49 test_dolt
drwxr-xr-x  2 root root 4.0K Oct 29 20:49 test_tmp

root@illi:/mnt# ll test_tmp/
total 8.0K
drwxr-xr-x 2 root root 4.0K Oct 29 20:49 .
drwxr-xr-x 4 root root 4.0K Oct 29 20:49 ..
-rw-r--r-- 1 root root    0 Oct 29 20:49 2c041878-863e-40f3-ae75-05e539f0b26f
-rw-r--r-- 1 root root    0 Oct 29 20:49 2ea46c47-cac1-4501-95e5-1fad234e2c44

root@illi:/mnt#

I don't see any mention of multiple mounted file systems in your description. Can you confirm one way or another? I suspect if you attempt to run dolt sql-server --config... in the same file system as where your data dir is you will stop seeing the .tmp artifact.

This is a playground environment of a minimal Alpine linux. I haven't partitioned the system in anyway as it's a virtual environment - only the standard /boot, tmp, and swap partitions exist next to the root one:

root@illi:/opt# cat /etc/fstab
UUID=642ffc8a-0a7a-4dea-9186-5e76c74f1fd1	/	ext4	rw,relatime 0 1
UUID=d0d49bb1-d880-40d7-a47f-90c1fa4f36f0	/boot	ext4	rw,relatime 0 2
UUID=6391df5b-f708-48a4-a3bf-5d6f9dee8a83	swap	swap	defaults	0 0
/dev/cdrom	/media/cdrom	iso9660	noauto,ro 0 0
/dev/usbdisk	/media/usb	vfat	noauto	0 0
tmpfs	/tmp	tmpfs	nosuid,nodev	0	0
root@illi:/opt#

I could be wrong but I consider /tmp being on its own partition common and generally better practice, since tmpfs file system was designed for handling temporary data from memory, instead of relying on a hard drive. Please don't get me wrong, I am not saying any created temporary data HAS TO BE in /tmp, I am just saying it would be better for users to be able to configure their location instead of getting them directly created it in the current working directory of the shell starting the dolt server.

For clarity, I have installed very few packages on this Alpine instance and I don't believe I have made any OS related modifications that could affect how the OS runs. If necessary, I can spin up another instance of the latest Alpine and test this on it - just let me know what/how you want me to test and I'll see what I can do to assist.


Note, that I only was able to do this by setting TMPDIR env var, but the main call out is that our test of moving "./testfile" is effectively forcing you to start the binary on the same file system where you will store your data files. That is certainly a bug, and I'll get on fixing that.

I played around a bit with the TMPDIR variable and I manually set it to point to different locations, here are the results:

  • when pointed to a partition with the same file system (ext4) Dolt is using the TMPDIR properly
  • when pointed to a partition with different file system (ext3 and tmpfs) Dolt "ignores it" and just creates .tmp in the current working directory
  • when pointed to a location on a partition with the same file system (ext4), but to a symlink to a different partition with a different file system ([ext4] /mnt/dolt => [tmpfs] /tmp/dolt) Dolt also "ignores it" and just creates .tmp in the current working directory

The config file I am using (the aforementioned db.conf) is boilerplate YAML, made of settings I found in the documentation, along with short description of each config parameter. I am attaching the config file, renamed from "db.conf" to "db.conf.txt", due to GitHub's file name restrictions.

db.conf.txt

Albeit, I don't think the config file matters much since the .tmp directory is created in the current working directory even when there are no settings defined, instead of being placed inside of the created .dolt directory (which I am given to understand is the data_dir parameter):

root@illi:/mnt# ll
total 8.0K
drwxr-xr-x  2 root root 4.0K Oct 29 20:27 .
drwxr-xr-x 24 root root 4.0K Oct 29 11:16 ..

root@illi:/mnt# /usr/local/bin/dolt sql-server
Starting server with Config HP="localhost:3306"|T="28800000"|R="false"|L="info"|S="/tmp/mysql.sock"
INFO[0000] Server ready. Accepting connections.
WARN[0000] secure_file_priv is set to "", which is insecure.
WARN[0000] Any user with GRANT FILE privileges will be able to read any file which the sql-server process can read.
WARN[0000] Please consider restarting the server with secure_file_priv set to a safe (or non-existent) directory.
^CINFO[0003] Server closing listener. No longer accepting connections.

root@illi:/mnt# ll
total 16K
drwxr-xr-x  4 root root 4.0K Oct 29 20:28 .
drwxr-xr-x 24 root root 4.0K Oct 29 11:16 ..
drwxr-xr-x  2 root root 4.0K Oct 29 20:28 .dolt
drwxr-xr-x  2 root root 4.0K Oct 29 20:28 .tmp

root@illi:/mnt#

So you are definitely right on pinpointing this to the file system difference! I just hope something can be done about Dolt properly using tmpfs by default, or at least allow for the temp files to be configurable so one can manually "force" Dolt to directly place them in a location like /tmp/dolt/.

Thank you for the time and effort, it is appreciated!

@macneale4
Copy link
Contributor

Thanks for confirming. The use of CWD when you specify a data dir is definitely wrong.

I agree that /tmp being on another partition is a good practice, and dolt should play nice with others.

One thing that makes this a little trickier is that some Dolt temp files need to be renamed to place them into our storage system. These files must be on the same partition as the data dir. Other files should just go in /tmp or TMPDIR, as you would expect. I need to refactor a bit to make the two categories clear.

Regardless, I think I have a good idea of what is causing your pain and I'll have a fix out in the next day or so to address it.

@macneale4
Copy link
Contributor

Heads up that I'm still working on this. There are actually a couple bugs in play here, both of which need a little care.

In the meantime, you can work around this by specifying the --data-dir option on the command line arguments instead of in the config file. Specifically, you'll want to do it before the sql-server sub command token. Something like:

start-stop-daemon --start --background --make-pidfile --pidfile $pidfile --exec $cmd -- --data-dir ${XYZ} sql-server --config /opt/db.conf

Having the argument duplicated in the config file will not cause harm, but will slow start up because we'll load the DB twice. The change I'm working on will discover and use the data dir option in the config file earlier in startup processing. It will also error out if there is ambiguous configuration. Until I have that out for you, I suggest you use the --data-dir argument.

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

No branches or pull requests

5 participants