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

Add tool to remove grouping from report results #17589

Merged
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
63 changes: 63 additions & 0 deletions tools/remove_grouping_from_report_results.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
#!/usr/bin/env ruby
require 'trollop'
Copy link
Member

Choose a reason for hiding this comment

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

require 'optparse' for lyfe!!1! :trollface:

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm waiting for @kbrock to rename trollor to optimal or whatever we decided. 🕐

Copy link
Member Author

Choose a reason for hiding this comment

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

or trollop

Copy link
Member

Choose a reason for hiding this comment

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

I always favor stdlib things when possible, so that was what my comment was all about.


opts = Trollop.options do
banner "Remove extras[:grouping] from MiqReportResult#report column.\n\nUsage: ruby #{$PROGRAM_NAME} [options]\n\nOptions:\n\t"
opt :dry_run, "For testing, rollback any changes when the script exits.", :short => :none, :default => false
opt :batch_size, "Limit memory usage by process this number of report results at a time.", :default => 50
Copy link
Member Author

Choose a reason for hiding this comment

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

Does 50 batch size make sense? Considering the size of these, I wanted extra queries instead of possibly bloating the memory of the process.

Copy link
Member

Choose a reason for hiding this comment

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

I think so. The deserialization is probably going to hit us harder than the queries time themselves. I don't see this as being an issue with N+1's.

opt :count, "Stop checking after this number of report results.", :default => 0
end

puts "Using options: #{opts.inspect}\n\n"

if defined?(Rails)
puts "Warning: Rails is already loaded! Please do not invoke using rails runner. Exiting with help text.\n\n"
Trollop.educate
Copy link
Member Author

Choose a reason for hiding this comment

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

@NickLaMuro changed the -- handling to just warn, print the help text and exit. Let me know if this is overkill.

Copy link
Member

Choose a reason for hiding this comment

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

Works for me. I didn't realize what you had was standard for the tools/ dir, so it was more of a "is this okay" out of naivety than anything else.

end

# Load rails after trollop options are set. No one wants to wait for -h.
require File.expand_path('../config/environment', __dir__)

# Wrap all changes in a transaction and roll them back if dry-run or an error occurs.
ActiveRecord::Base.connection.begin_transaction(:joinable => false)

if opts[:dry_run]
puts "Running in dry-run, all changes will be rolled back when complete."

at_exit do
Copy link
Member

Choose a reason for hiding this comment

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

Just because I am not sure: I assume that at_exit will still fire after a exit 1, right?

(Nick could probably just go test this in a ruby script himself...)

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, good point. I'll test that

Copy link
Member

Choose a reason for hiding this comment

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

No need, I already did:

$ ruby -e "at_exit { puts 'foo' }; exit 42"
foo
$ echo $?
42

Copy link
Member Author

Choose a reason for hiding this comment

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

I did several tests of dry run so I'm pretty confident it works. It's a little messy how I'm doing it right now but I can't think of a better way to handle the begin/commit/rollback for dry-run/error/exit

ActiveRecord::Base.connection.rollback_transaction
end
end

start = Time.now.utc
total = 0
fixed = 0

MiqReportResult.find_each(:batch_size => opts[:batch_size]).with_index do |rr, i|
begin
break if opts[:count].positive? && i == opts[:count]
total += 1

next if rr.report.nil? || rr.report.extras.nil?

if rr.report.extras.key?(:grouping)
rr.report.extras.except!(:grouping)
rr.save!
if rr.reload.report.extras.key?(:grouping)
Copy link
Member

Choose a reason for hiding this comment

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

When would this happen?

Copy link
Member Author

Choose a reason for hiding this comment

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

This was here before I changed line 43 from .save to .save! and added the begin/rescue around it. It's unlikely to happen now but I left this because it seemed odd to leave out one of the three cases below:

"doesn't need fixing"
"fixed"
"could NOT be fixed"

puts "MiqReportResult: #{rr.id} could NOT be fixed"
else
puts "MiqReportResult: #{rr.id} fixed"
fixed += 1
end
else
puts "MiqReportResult: #{rr.id} doesn't need fixing"
end
rescue => err
puts "\nWarning: Rolling back all changes since an error occurred on MiqReportResult with id: #{rr.try(:id)}: #{err.message}"
ActiveRecord::Base.connection.rollback_transaction unless opts[:dry_run]
exit 1
end
end

ActiveRecord::Base.connection.commit_transaction unless opts[:dry_run]
puts "\nProcessed #{total} rows. #{fixed} were fixed. #{Time.now.utc - start} seconds"