Skip to content

Use sp_return_logs for billing reports, not monthly table #6935

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.

Sign up for GitHub

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

Merged
zachmargolis merged 9 commits into from
Sep 15, 2022
Merged

Use sp_return_logs for billing reports, not monthly table #6935

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
7d39180
Rewrite total-monthly-auths-report to use sp_return_logs
zachmargolis Sep 9, 2022
135d707
Update combined-invoice-supplement report to use sp_return_logs
zachmargolis Sep 9, 2022
9cda538
Update MonthHelper to not subtract one, most contracts don't "overlap"
zachmargolis Sep 9, 2022
6a3b737
Fix a few specs to use the right tables
zachmargolis Sep 9, 2022
70ae9c6
Add manual retry + rollback
zachmargolis Sep 9, 2022
fbc1675
Add changelog
zachmargolis Sep 9, 2022
3f12f17
Fix a few spec stragglers
zachmargolis Sep 9, 2022
25d6e2d
Merge remote-tracking branch 'origin/main' into margolis-plain-return…
zachmargolis Sep 13, 2022
c5b005e
Undo switch to timestamps, turns out ::date truncation does in fact
zachmargolis Sep 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions app/jobs/reports/month_helper.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,5 @@ def months(date_range)

results
end

def full_month?(date_range)
date_range.begin == date_range.begin.beginning_of_month &&
date_range.end == date_range.end.end_of_month
end
end
end
70 changes: 23 additions & 47 deletions app/services/db/monthly_sp_auth_count/total_monthly_auth_counts_within_iaa_window.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,32 +19,31 @@ def call(issuer:, iaa:, iaa_start_date:, iaa_end_date:)

iaa_range = (iaa_start_date..iaa_end_date)

full_months, partial_months = Reports::MonthHelper.months(iaa_range).
partition do |month_range|
Reports::MonthHelper.full_month?(month_range)
end

# The subqueries create a uniform representation of data:
# - full months from monthly_sp_auth_counts
# - partial months by aggregating sp_return_logs
# Query a month at a time, to keep query time/result size fairly reasonable
# The results are rows with [user_id, ial, auth_count, year_month]
queries = [
*full_month_subquery(issuer: issuer, full_months: full_months),
*partial_month_subqueries(issuer: issuer, partial_months: partial_months),
]
months = Reports::MonthHelper.months(iaa_range)
queries = build_queries(issuer: issuer, months: months)

ial_to_year_month_to_users = Hash.new do |ial_h, ial_k|
ial_h[ial_k] = Hash.new { |ym_h, ym_k| ym_h[ym_k] = Multiset.new }
end

queries.each do |query|
stream_query(query) do |row|
user_id = row['user_id']
year_month = row['year_month']
auth_count = row['auth_count']
ial = row['ial']

ial_to_year_month_to_users[ial][year_month].add(user_id, auth_count)
temp_copy = ial_to_year_month_to_users.deep_dup

with_retries(
max_tries: 3,
rescue: PG::TRSerializationFailure,
handler: proc { ial_to_year_month_to_users = temp_copy },
) do
Comment on lines +32 to +38
Copy link
Copy Markdown
Contributor Author

@zachmargolis zachmargolis Sep 9, 2022

Choose a reason for hiding this comment

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

see commit notes (70ae9c6) for a longer comment, but short version is we get these errors occasionally and I figured it was worth a shot seeing if we could quickly retry + recover rather than abort the entire job

Copy link
Copy Markdown
Contributor Author

@zachmargolis zachmargolis Sep 9, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

and in case it's not obvious what it's doing, we have a ruby nested hash/multiset thing object where we add results incrementally and this is creating a copy before streaming the results, and then restoring the copy to the last known good result every time the query fails

we could rewrite this as begin/rescue syntax but we'd need add some sleep code and need a retry counter ourself, so this seemed clear enough? the alternative would be:

temp_copy = ial_to_year_month_to_users.deep_dup
attempt_count = 0
begin
  stream_query(query) do |row|
    # ...
  end
rescue PG::TRSerializationFailure => e
  attempt_count += 1
  if attempt_count <3
    ial_to_year_month_to_users = temp_copy
    retry
   else
    raise e
  end
end

stream_query(query) do |row|
user_id = row['user_id']
year_month = row['year_month']
auth_count = row['auth_count']
ial = row['ial']

ial_to_year_month_to_users[ial][year_month].add(user_id, auth_count)
end
end
end

Expand Down Expand Up @@ -81,35 +80,12 @@ def call(issuer:, iaa:, iaa_start_date:, iaa_end_date:)
rows
end

# @return [String]
def full_month_subquery(issuer:, full_months:)
return if full_months.blank?
params = {
issuer: issuer,
year_months: full_months.map { |r| r.begin.strftime('%Y%m') },
}.transform_values { |value| quote(value) }

format(<<~SQL, params)
SELECT
monthly_sp_auth_counts.user_id
, monthly_sp_auth_counts.year_month
, monthly_sp_auth_counts.ial
, SUM(monthly_sp_auth_counts.auth_count)::bigint AS auth_count
FROM
monthly_sp_auth_counts
WHERE
monthly_sp_auth_counts.issuer = %{issuer}
AND monthly_sp_auth_counts.year_month IN %{year_months}
GROUP BY
monthly_sp_auth_counts.year_month
, monthly_sp_auth_counts.ial
, monthly_sp_auth_counts.user_id
SQL
end

# @param [String] issuer
# @param [Array<Range<Date>>] months ranges of dates by month that are included in this iaa,
# the first and last may be partial months
# @return [Array<String>]
def partial_month_subqueries(issuer:, partial_months:)
partial_months.map do |month_range|
def build_queries(issuer:, months:)
months.map do |month_range|
params = {
range_start: month_range.begin,
range_end: month_range.end,
Expand Down
66 changes: 23 additions & 43 deletions app/services/db/monthly_sp_auth_count/unique_monthly_auth_counts_by_iaa.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,32 +15,31 @@ def call(key:, issuers:, start_date:, end_date:)

return [] if !date_range || issuers.blank?

full_months, partial_months = Reports::MonthHelper.months(date_range).
partition do |month_range|
Reports::MonthHelper.full_month?(month_range)
end

# The subqueries create a uniform representation of data:
# - full months from monthly_sp_auth_counts
# - partial months by aggregating sp_return_logs
# Query a month at a time, to keep query time/result size fairly reasonable
# The results are rows with [user_id, ial, auth_count, year_month]
queries = [
*full_month_subquery(issuers: issuers, full_months: full_months),
*partial_month_subqueries(issuers: issuers, partial_months: partial_months),
]
months = Reports::MonthHelper.months(date_range)
queries = build_queries(issuers: issuers, months: months)

ial_to_year_month_to_users = Hash.new do |ial_h, ial_k|
ial_h[ial_k] = Hash.new { |ym_h, ym_k| ym_h[ym_k] = Multiset.new }
end

queries.each do |query|
stream_query(query) do |row|
user_id = row['user_id']
year_month = row['year_month']
auth_count = row['auth_count']
ial = row['ial']

ial_to_year_month_to_users[ial][year_month].add(user_id, auth_count)
temp_copy = ial_to_year_month_to_users.deep_dup

with_retries(
max_tries: 3,
rescue: PG::TRSerializationFailure,
handler: proc { ial_to_year_month_to_users = temp_copy },
) do
stream_query(query) do |row|
user_id = row['user_id']
year_month = row['year_month']
auth_count = row['auth_count']
ial = row['ial']

ial_to_year_month_to_users[ial][year_month].add(user_id, auth_count)
end
end
end

Expand Down Expand Up @@ -76,31 +75,12 @@ def call(key:, issuers:, start_date:, end_date:)
rows
end

# @return [String]
def full_month_subquery(issuers:, full_months:)
return if full_months.blank?
params = {
issuers: issuers,
year_months: full_months.map { |r| r.begin.strftime('%Y%m') },
}.transform_values { |value| quote(value) }

format(<<~SQL, params)
SELECT
monthly_sp_auth_counts.user_id
, monthly_sp_auth_counts.year_month
, monthly_sp_auth_counts.auth_count
, monthly_sp_auth_counts.ial
FROM
monthly_sp_auth_counts
WHERE
monthly_sp_auth_counts.issuer IN %{issuers}
AND monthly_sp_auth_counts.year_month IN %{year_months}
SQL
end

# @param [Array<String>] issuers all the issuers for this iaa
# @param [Array<Range<Date>>] months ranges of dates by month that are included in this iaa,
# the first and last may be partial months
# @return [Array<String>]
def partial_month_subqueries(issuers:, partial_months:)
partial_months.map do |month_range|
def build_queries(issuers:, months:)
months.map do |month_range|
params = {
range_start: month_range.begin,
range_end: month_range.end,
Expand Down
42 changes: 29 additions & 13 deletions spec/jobs/reports/agency_invoice_iaa_supplement_report_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,24 +102,28 @@

# 1 unique user in month 1 at IAA 2 @ IAL 2
create(
:monthly_sp_auth_count,
:sp_return_log,
user_id: user1.id,
auth_count: 1,
ial: 2,
issuer: iaa2_sp.issuer,
year_month: inside_iaa2.strftime('%Y%m'),
requested_at: inside_iaa2,
returned_at: inside_iaa2,
billable: true,
)

# 2 users, each 2 auths (1 unique) in month 2 at IAA 2 @ IAL 2
[user1, user2].each do |user|
create(
:monthly_sp_auth_count,
user_id: user.id,
auth_count: 2,
ial: 2,
issuer: iaa2_sp.issuer,
year_month: (inside_iaa2 + 1.month).strftime('%Y%m'),
)
2.times do
create(
:sp_return_log,
user_id: user.id,
ial: 2,
issuer: iaa2_sp.issuer,
requested_at: inside_iaa2 + 1.month,
returned_at: inside_iaa2 + 1.month,
billable: true,
)
end
end
end

Expand Down Expand Up @@ -188,7 +192,7 @@
let(:iaa2_key) { "#{gtc1.gtc_number}-#{format('%04d', iaa_order2.order_number)}" }

let(:iaa1_range) { Date.new(2020, 4, 15)..Date.new(2021, 4, 14) }
let(:iaa2_range) { Date.new(2021, 4, 14)..Date.new(2022, 4, 13) }
let(:iaa2_range) { Date.new(2021, 4, 15)..Date.new(2022, 4, 14) }
it 'counts up costs by issuer + ial, and includes iaa and app_id' do
results = JSON.parse(report.perform(Time.zone.today), symbolize_names: true)

Expand All @@ -205,14 +209,26 @@
iaa_start_date: iaa1_range.begin.to_s,
iaa_end_date: iaa1_range.end.to_s,
},
{
iaa: iaa2_key,
ial1_total_auth_count: 0,
ial2_total_auth_count: 1,
ial1_unique_users: 0,
ial2_unique_users: 1,
ial1_new_unique_users: 0,
ial2_new_unique_users: 1,
year_month: inside_iaa2.strftime('%Y%m'),
iaa_start_date: iaa2_range.begin.to_s,
iaa_end_date: iaa2_range.end.to_s,
},
Comment on lines +212 to +223
Copy link
Copy Markdown
Contributor Author

@zachmargolis zachmargolis Sep 9, 2022

Choose a reason for hiding this comment

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

I think this is correct now that this has another section... I think it may have been grouped inaccurately before

{
iaa: iaa2_key,
ial1_total_auth_count: 0,
ial2_total_auth_count: 4,
ial1_unique_users: 0,
ial2_unique_users: 2,
ial1_new_unique_users: 0,
ial2_new_unique_users: 2,
ial2_new_unique_users: 1,
year_month: (inside_iaa2 + 1.month).strftime('%Y%m'),
iaa_start_date: iaa2_range.begin.to_s,
iaa_end_date: iaa2_range.end.to_s,
Expand Down
42 changes: 24 additions & 18 deletions spec/jobs/reports/agency_invoice_issuer_supplement_report_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,22 +21,28 @@
end

before do
create(
:monthly_sp_auth_count,
user: user,
service_provider: sp,
ial: 1,
auth_count: 11,
year_month: iaa_range.begin.strftime('%Y%m'),
)
create(
:monthly_sp_auth_count,
user: user,
service_provider: sp,
ial: 2,
auth_count: 22,
year_month: iaa_range.begin.strftime('%Y%m'),
)
3.times do
create(
:sp_return_log,
user: user,
service_provider: sp,
ial: 1,
requested_at: iaa_range.begin + 1.day,
returned_at: iaa_range.begin + 1.day,
billable: true,
)
end
4.times do
create(
:sp_return_log,
user: user,
service_provider: sp,
ial: 2,
requested_at: iaa_range.begin + 1.day,
returned_at: iaa_range.begin + 1.day,
billable: true,
)
end
end

it 'totals up auth counts within IAA window by month' do
Expand All @@ -50,8 +56,8 @@
iaa_start_date: '2021-01-01',
iaa_end_date: '2021-12-31',
year_month: '202101',
ial1_total_auth_count: 11,
ial2_total_auth_count: 22,
ial1_total_auth_count: 3,
ial2_total_auth_count: 4,
ial1_unique_users: 1,
ial2_unique_users: 1,
},
Expand Down
14 changes: 8 additions & 6 deletions spec/jobs/reports/combined_invoice_supplement_report_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,22 +112,24 @@

# 1 unique user in month 1 at IAA 2 sp 1 @ IAL 2
create(
:monthly_sp_auth_count,
:sp_return_log,
user_id: user1.id,
auth_count: 1,
ial: 2,
issuer: iaa2_sp1.issuer,
year_month: inside_iaa2.strftime('%Y%m'),
requested_at: inside_iaa2,
returned_at: inside_iaa2,
billable: true,
)

# 1 unique user in month 1 at IAA 2 sp 2 @ IAL 2
create(
:monthly_sp_auth_count,
:sp_return_log,
user_id: user2.id,
auth_count: 1,
ial: 2,
issuer: iaa2_sp2.issuer,
year_month: inside_iaa2.strftime('%Y%m'),
requested_at: inside_iaa2,
returned_at: inside_iaa2,
billable: true,
)
end

Expand Down
10 changes: 0 additions & 10 deletions spec/jobs/reports/month_helper_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,4 @@
)
end
end

describe '.full_month?' do
it 'is true when the range is a full month' do
expect(full_month?(Date.new(2021, 1, 1)..Date.new(2021, 1, 31))).to eq(true)
end

it 'is false when the range is not a full month' do
expect(full_month?(Date.new(2021, 1, 2)..Date.new(2021, 1, 30))).to eq(false)
end
end
end
Loading