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

Fix --clear-db on Windows #7474

Merged
merged 9 commits into from
Oct 9, 2020

Conversation

dv8silencer
Copy link
Contributor

What type of PR is this?

Bug fix

What does this PR do? Why is it needed?
Fixes the issue where --clear-db in Windows leads to an error quoted in #7401 :

WARN node: Removing database
ERROR main: could not clear database: could not remove database file: remove C:\Users\Username\AppData\Roaming\Eth2\beaconchaindata/beaconchain.db: The process cannot access the file because it is being used by another process.
ERROR FAILED

This is in the validator, beacon-chain, as well as the slasher.

This is because on Linux, you can indeed os.Remove a file in use without the OS complaining-- this unlinks it from the containing directory but since a process has it open, the actual data on disk will remain until the process ends. At the time the process exits, if there is no other reference to that data, it'll be moved to the chopping block. On Windows, you cannot delete a file in use. In master, the clearing DB code calls a function to delete the .db file and then soon afterwards, creates a new DB [file] which will indeed persist after the process ends. But before it does the delete, it has a database with that file to-be-deleted DB file still open.

Example in the beacon-chain:

	if clearDBConfirmed || forceClearDB {
		log.Warning("Removing database")
		if err := d.ClearDB(); err != nil {
			return errors.Wrap(err, "could not clear database")
		}
		d, err = db.NewDB(dbPath, b.stateSummaryCache)
		if err != nil {
			return errors.Wrap(err, "could not create new database")
		}
	}

Which issues(s) does this PR fix?

Fixes #7401

Other notes for review

@dv8silencer dv8silencer requested a review from a team as a code owner October 9, 2020 03:19
@codecov
Copy link

codecov bot commented Oct 9, 2020

Codecov Report

Merging #7474 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #7474   +/-   ##
=======================================
  Coverage   60.60%   60.60%           
=======================================
  Files         424      424           
  Lines       30459    30459           
=======================================
  Hits        18461    18461           
  Misses       8989     8989           
  Partials     3009     3009           

Copy link
Contributor

@farazdagi farazdagi left a comment

Choose a reason for hiding this comment

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

Thank you, looks great!

@prylabs-bulldozer prylabs-bulldozer bot merged commit c944f29 into prysmaticlabs:master Oct 9, 2020
@dv8silencer dv8silencer deleted the dv8-i7401 branch October 9, 2020 16:35
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 this pull request may close these issues.

Clear Database does not work on Windows
2 participants