Skip to content

Commit

Permalink
invoice: overall HA improvements
Browse files Browse the repository at this point in the history 
* Take the (parent) account lock when processing children invoices events
* Improve logging around parent invoice generation
* Improve performance by looking-up parent invoices in bulk for a given set
  of child invoices

Signed-off-by: Pierre-Alexandre Meyer <pierre@mouraf.org>
  • Loading branch information
@pierre
pierre committed Feb 23, 2017
1 parent 3e25bcd commit 43d6370
Show file tree
Hide file tree
Showing 10 changed files with 162 additions and 62 deletions.
55 changes: 43 additions & 12 deletions invoice/src/main/java/org/killbill/billing/invoice/InvoiceDispatcher.java
View file
Expand Up @@ -771,56 +771,73 @@ public String toString() {
} }
} }


public void processParentInvoiceForInvoiceGeneration(final Account account, final UUID childInvoiceId, final InternalCallContext context) throws InvoiceApiException { public void processParentInvoiceForInvoiceGeneration(final Account childAccount, final UUID childInvoiceId, final InternalCallContext context) throws InvoiceApiException {
GlobalLock lock = null;
try {
lock = locker.lockWithNumberOfTries(LockerType.ACCNT_INV_PAY.toString(), childAccount.getParentAccountId().toString(), invoiceConfig.getMaxGlobalLockRetries());

processParentInvoiceForInvoiceGenerationWithLock(childAccount, childInvoiceId, context);
} catch (final LockFailedException e) {
log.warn("Failed to process parent invoice for parentAccountId='{}'", childAccount.getParentAccountId().toString(), e);
} finally {
if (lock != null) {
lock.release();
}
}
}


private void processParentInvoiceForInvoiceGenerationWithLock(final Account childAccount, final UUID childInvoiceId, final InternalCallContext context) throws InvoiceApiException {
log.info("Processing parent invoice for parentAccountId='{}', childInvoiceId='{}'", childAccount.getParentAccountId(), childInvoiceId);
final InvoiceModelDao childInvoiceModelDao = invoiceDao.getById(childInvoiceId, context); final InvoiceModelDao childInvoiceModelDao = invoiceDao.getById(childInvoiceId, context);
final Invoice childInvoice = new DefaultInvoice(childInvoiceModelDao); final Invoice childInvoice = new DefaultInvoice(childInvoiceModelDao);


final Long parentAccountRecordId = internalCallContextFactory.getRecordIdFromObject(account.getParentAccountId(), ObjectType.ACCOUNT, buildTenantContext(context)); final Long parentAccountRecordId = internalCallContextFactory.getRecordIdFromObject(childAccount.getParentAccountId(), ObjectType.ACCOUNT, buildTenantContext(context));
final InternalCallContext parentContext = internalCallContextFactory.createInternalCallContext(parentAccountRecordId, context); final InternalCallContext parentContext = internalCallContextFactory.createInternalCallContext(parentAccountRecordId, context);


BigDecimal childInvoiceAmount = InvoiceCalculatorUtils.computeChildInvoiceAmount(childInvoice.getCurrency(), childInvoice.getInvoiceItems()); BigDecimal childInvoiceAmount = InvoiceCalculatorUtils.computeChildInvoiceAmount(childInvoice.getCurrency(), childInvoice.getInvoiceItems());
InvoiceModelDao draftParentInvoice = invoiceDao.getParentDraftInvoice(account.getParentAccountId(), parentContext); InvoiceModelDao draftParentInvoice = invoiceDao.getParentDraftInvoice(childAccount.getParentAccountId(), parentContext);


final String description = account.getExternalKey().concat(" summary"); final String description = childAccount.getExternalKey().concat(" summary");
if (draftParentInvoice != null) { if (draftParentInvoice != null) {

for (InvoiceItemModelDao item : draftParentInvoice.getInvoiceItems()) { for (InvoiceItemModelDao item : draftParentInvoice.getInvoiceItems()) {
if ((item.getChildAccountId() != null) && item.getChildAccountId().equals(childInvoice.getAccountId())) { if ((item.getChildAccountId() != null) && item.getChildAccountId().equals(childInvoice.getAccountId())) {
// update child item amount for existing parent invoice item // update child item amount for existing parent invoice item
BigDecimal newChildInvoiceAmount = childInvoiceAmount.add(item.getAmount()); BigDecimal newChildInvoiceAmount = childInvoiceAmount.add(item.getAmount());
log.info("Updating existing itemId='{}', oldAmount='{}', newAmount='{}' on existing DRAFT invoiceId='{}'", item.getId(), item.getAmount(), newChildInvoiceAmount, draftParentInvoice.getId());
invoiceDao.updateInvoiceItemAmount(item.getId(), newChildInvoiceAmount, parentContext); invoiceDao.updateInvoiceItemAmount(item.getId(), newChildInvoiceAmount, parentContext);
return; return;
} }
} }


// new item when the parent invoices does not have this child item yet // new item when the parent invoices does not have this child item yet
final ParentInvoiceItem newParentInvoiceItem = new ParentInvoiceItem(UUID.randomUUID(), context.getCreatedDate(), draftParentInvoice.getId(), account.getParentAccountId(), account.getId(), childInvoiceAmount, account.getCurrency(), description); final ParentInvoiceItem newParentInvoiceItem = new ParentInvoiceItem(UUID.randomUUID(), context.getCreatedDate(), draftParentInvoice.getId(), childAccount.getParentAccountId(), childAccount.getId(), childInvoiceAmount, childAccount.getCurrency(), description);
draftParentInvoice.addInvoiceItem(new InvoiceItemModelDao(newParentInvoiceItem)); final InvoiceItemModelDao parentInvoiceItem = new InvoiceItemModelDao(newParentInvoiceItem);
draftParentInvoice.addInvoiceItem(parentInvoiceItem);


List<InvoiceModelDao> invoices = new ArrayList<InvoiceModelDao>(); List<InvoiceModelDao> invoices = new ArrayList<InvoiceModelDao>();
invoices.add(draftParentInvoice); invoices.add(draftParentInvoice);
log.info("Adding new itemId='{}', amount='{}' on existing DRAFT invoiceId='{}'", parentInvoiceItem.getId(), childInvoiceAmount, draftParentInvoice.getId());
invoiceDao.createInvoices(invoices, parentContext); invoiceDao.createInvoices(invoices, parentContext);
} else { } else {
if (shouldIgnoreChildInvoice(childInvoice, childInvoiceAmount)) { if (shouldIgnoreChildInvoice(childInvoice, childInvoiceAmount)) {
return; return;
} }


final LocalDate invoiceDate = context.toLocalDate(context.getCreatedDate()); final LocalDate invoiceDate = context.toLocalDate(context.getCreatedDate());
draftParentInvoice = new InvoiceModelDao(account.getParentAccountId(), invoiceDate, account.getCurrency(), InvoiceStatus.DRAFT, true); draftParentInvoice = new InvoiceModelDao(childAccount.getParentAccountId(), invoiceDate, childAccount.getCurrency(), InvoiceStatus.DRAFT, true);
final InvoiceItem parentInvoiceItem = new ParentInvoiceItem(UUID.randomUUID(), context.getCreatedDate(), draftParentInvoice.getId(), account.getParentAccountId(), account.getId(), childInvoiceAmount, account.getCurrency(), description); final InvoiceItem parentInvoiceItem = new ParentInvoiceItem(UUID.randomUUID(), context.getCreatedDate(), draftParentInvoice.getId(), childAccount.getParentAccountId(), childAccount.getId(), childInvoiceAmount, childAccount.getCurrency(), description);
draftParentInvoice.addInvoiceItem(new InvoiceItemModelDao(parentInvoiceItem)); draftParentInvoice.addInvoiceItem(new InvoiceItemModelDao(parentInvoiceItem));


// build account date time zone // build account date time zone
final FutureAccountNotifications futureAccountNotifications = new FutureAccountNotifications(ImmutableMap.<UUID, List<SubscriptionNotification>>of()); final FutureAccountNotifications futureAccountNotifications = new FutureAccountNotifications(ImmutableMap.<UUID, List<SubscriptionNotification>>of());


log.info("Adding new itemId='{}', amount='{}' on new DRAFT invoiceId='{}'", parentInvoiceItem.getId(), childInvoiceAmount, draftParentInvoice.getId());
invoiceDao.createInvoice(draftParentInvoice, draftParentInvoice.getInvoiceItems(), true, futureAccountNotifications, parentContext); invoiceDao.createInvoice(draftParentInvoice, draftParentInvoice.getInvoiceItems(), true, futureAccountNotifications, parentContext);
} }


// save parent child invoice relation // save parent child invoice relation
final InvoiceParentChildModelDao invoiceRelation = new InvoiceParentChildModelDao(draftParentInvoice.getId(), childInvoiceId, account.getId()); final InvoiceParentChildModelDao invoiceRelation = new InvoiceParentChildModelDao(draftParentInvoice.getId(), childInvoiceId, childAccount.getId());
invoiceDao.createParentChildInvoiceRelation(invoiceRelation, parentContext); invoiceDao.createParentChildInvoiceRelation(invoiceRelation, parentContext);

} }


private boolean shouldIgnoreChildInvoice(final Invoice childInvoice, final BigDecimal childInvoiceAmount) { private boolean shouldIgnoreChildInvoice(final Invoice childInvoice, final BigDecimal childInvoiceAmount) {
Expand All @@ -840,8 +857,22 @@ private boolean shouldIgnoreChildInvoice(final Invoice childInvoice, final BigDe
return true; return true;
} }


public void processParentInvoiceForAdjustments(final AccountData account, final UUID childInvoiceId, final InternalCallContext context) throws InvoiceApiException { public void processParentInvoiceForAdjustments(final Account childAccount, final UUID childInvoiceId, final InternalCallContext context) throws InvoiceApiException {
GlobalLock lock = null;
try {
lock = locker.lockWithNumberOfTries(LockerType.ACCNT_INV_PAY.toString(), childAccount.getParentAccountId().toString(), invoiceConfig.getMaxGlobalLockRetries());

processParentInvoiceForAdjustmentsWithLock(childAccount, childInvoiceId, context);
} catch (final LockFailedException e) {
log.warn("Failed to process parent invoice for parentAccountId='{}'", childAccount.getParentAccountId().toString(), e);
} finally {
if (lock != null) {
lock.release();
}
}
}


public void processParentInvoiceForAdjustmentsWithLock(final Account account, final UUID childInvoiceId, final InternalCallContext context) throws InvoiceApiException {
final InvoiceModelDao childInvoiceModelDao = invoiceDao.getById(childInvoiceId, context); final InvoiceModelDao childInvoiceModelDao = invoiceDao.getById(childInvoiceId, context);
final InvoiceModelDao parentInvoiceModelDao = childInvoiceModelDao.getParentInvoice(); final InvoiceModelDao parentInvoiceModelDao = childInvoiceModelDao.getParentInvoice();


Expand Down
1 change: 0 additions & 1 deletion invoice/src/main/java/org/killbill/billing/invoice/InvoiceListener.java
View file
Expand Up @@ -83,7 +83,6 @@ public void handleSubscriptionTransition(final EffectiveSubscriptionInternalEven
} }
} }



@AllowConcurrentEvents @AllowConcurrentEvents
@Subscribe @Subscribe
public void handleBlockingStateTransition(final BlockingTransitionInternalEvent event) { public void handleBlockingStateTransition(final BlockingTransitionInternalEvent event) {
Expand Down

4 comments on commit 43d6370

@sbrossie
Copy link
Member

Choose a reason for hiding this comment

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

👍

Looks good overall, i am not sure this will be enough though. I guess this depends on your findings when running load tests.

@pierre
Copy link
Member Author

@pierre pierre commented on 43d6370 Feb 23, 2017

Choose a reason for hiding this comment

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

FWIW, for the following scenario:

  • Laptop (single node) setup with 5 bus threads and 20 notification threads
  • 25 parents and 1250 children (i.e. 50 per parent)
  • Each child has a monthly subscription, all children share the same BCD

Numbers after 18 months:

  • Parents invoices generation takes 4s
  • Children invoices generation takes 120s (10/s)

@sbrossie
Copy link
Member

Choose a reason for hiding this comment

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

That starts to look good!

  • I'd be interested to know more about the settings though ( 5 bus threads and 20 notification threads).
  • What is the next bottleneck (i don't necessarily think we need to fix it, but would be nice to know)?

@pierre
Copy link
Member Author

@pierre pierre commented on 43d6370 Feb 23, 2017

Choose a reason for hiding this comment

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

I'd be interested to know more about the settings though (5 bus threads and 20 notification threads).

👍 Let's chat offline.

What is the next bottleneck (i don't necessarily think we need to fix it, but would be nice to know)?

Still going through this. One interesting note is that each month adds about 3s-4s in the generation time (i.e. we start at 50s or 25 invoices/s then 53s or 23 invoices/s, etc.), so it sounds like refetching all invoice items has a linear impact.

Please sign in to comment.