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 clean etc #104

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
43 changes: 29 additions & 14 deletions exe/solr_wrapper
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,11 @@ options = {}
collection_options = {}
subtext = <<HELP
Commonly used command are:
clean : cleans all data from solr and configures a clean instance based on configuration options
dir : prints the solr instance dir
clean : stop and clean data from solr, then create a new instance
purge : as in 'clean', but this also removes solr download files
dir : prints the solr instance dir
status : information about running solr cores
stop : stop a running solr core
See 'solr_wrapper COMMAND --help' for more information on a specific command.
HELP

Expand Down Expand Up @@ -73,12 +76,21 @@ end


subcommands = {
'clean' => OptionParser.new do |opts|
opts.banner = "Usage: clean"
end,
'dir' => OptionParser.new do |opts|
opts.banner = "Usage: dir"
end,
'clean' => OptionParser.new do |opts|
opts.banner = "Usage: clean"
end,
'purge' => OptionParser.new do |opts|
opts.banner = "Usage: purge"
end,
'dir' => OptionParser.new do |opts|
opts.banner = "Usage: dir"
end,
'status' => OptionParser.new do |opts|
opts.banner = "Usage: status"
end,
'stop' => OptionParser.new do |opts|
opts.banner = "Usage: stop"
end,
}


Expand All @@ -100,14 +112,17 @@ instance = SolrWrapper.instance(options)

case command
when 'clean'
if instance.started?
$stderr.puts "Please stop solr before cleaning"
exit 1
end
$stderr.puts "cleaning #{instance.instance_dir}..."
instance.remove_instance_dir!
$stderr.puts "cleaning solr instance: #{instance.instance_dir} ..."
instance.clean!
Copy link
Owner

Choose a reason for hiding this comment

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

I'm not clear on why this change was made?

Copy link
Author

Choose a reason for hiding this comment

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

The clean! calls stop so that's the big diff. Given that, then it's not necessary to check if it's stopped, it will get stopped.

when 'purge'
$stderr.puts "cleaning solr downloads and solr instance: #{instance.instance_dir} ..."
instance.clean!(clean_downloads: true)
when 'dir'
puts instance.instance_dir
when 'status'
puts instance.status_info
when 'stop'
instance.stop
else
$stderr.print "Starting Solr #{instance.version} on port #{instance.port} ... "
instance.wrap do |conn|
Expand Down
50 changes: 28 additions & 22 deletions lib/solr_wrapper/instance.rb
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ def start
exec('start', p: port, c: config.cloud)

# Wait for solr to start
unless status
unless started?
sleep config.poll_interval
end

Expand All @@ -101,37 +101,41 @@ def restart
end

##
# Check the status of a managed Solr service
def status
return true unless config.managed?
# Is Solr running?
def started?
status_info.include? "running on port #{port}"
end

out = exec('status').read
out =~ /running on port #{port}/
rescue
false
##
# Check the status of a Solr service
alias status started?

##
# Get the status information for a Solr service
def status_info
exec('status').read
rescue => err
[
'No status information available',
"message: #{err.message}",
"stack trace: #{err.backtrace}"
].join("\n")
end

def pid
return unless config.managed?

@pid ||= begin
out = exec('status').read
out.match(/process (?<pid>\d+) running on port #{port}/) do |m|
status_info.match(/process (?<pid>\d+) running on port #{port}/) do |m|
m[:pid].to_i
end
end
rescue
nil
end

##
# Is Solr running?
def started?
!!status
end

def wait
while (Process.getpgid(pid) rescue status)
while (Process.getpgid(pid) rescue started?)
sleep config.poll_interval
end
end
Expand Down Expand Up @@ -222,14 +226,16 @@ def with_collection(options = {})
end

##
# Clean up any files solr_wrapper may have downloaded
def clean!
# Clean up any files solr_wrapper manages
def clean!(clean_downloads: false)
stop
remove_instance_dir!
FileUtils.remove_entry(config.download_dir, true) if File.exist?(config.download_dir)
Copy link
Owner

Choose a reason for hiding this comment

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

Instead of removing this line whole-sale, let's put it behind a named parameter flag (say, #clean! except_download_dir: true, or something better). There's a real use-case for "get rid of everything solr_wrapper has done", and I want to preserve that ability.

Copy link
Author

Choose a reason for hiding this comment

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

clean vs. clean! maybe? where the latter calls this clean and then also cleans up the downloads (all of them).

Copy link
Owner

Choose a reason for hiding this comment

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

A ! suffix usually indicates dangerousness or mutating state, neither of which seem to be true here. I'd rather we make it an opt-in/-out flag.

FileUtils.remove_entry(config.tmp_save_dir, true) if File.exist? config.tmp_save_dir
md5.clean!
FileUtils.remove_entry(config.version_file) if File.exist? config.version_file
FileUtils.remove_entry(config.version_file) if File.exist?(config.version_file)
if clean_downloads
md5.clean!
FileUtils.remove_file(config.solr_zip_path, true)
end
end

##
Expand Down
2 changes: 1 addition & 1 deletion lib/solr_wrapper/md5.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ def initialize(config)
end

def clean!
FileUtils.remove_entry(config.md5sum_path) if File.exist? config.md5sum_path
FileUtils.remove_file(config.md5sum_path, true)
end

def validate?(file)
Expand Down