From f8bc82084d739d67d81c98e4e675a7e00ccf41e4 Mon Sep 17 00:00:00 2001 From: Pierre-Alexandre Meyer Date: Mon, 23 Jan 2017 23:53:34 -0800 Subject: [PATCH 1/6] account: add fast search option The fast search option is triggered when limit == 1 and offset == -1. In that case, exact match queries only are performed and the first result is returned. This is to optimize the case when the user knows the email or name for example. Signed-off-by: Pierre-Alexandre Meyer --- .../billing/account/dao/AccountSqlDao.java | 5 ++ .../account/dao/DefaultAccountDao.java | 21 +++++++ .../billing/account/dao/AccountSqlDao.sql.stg | 60 ++++++++++++++++++- .../org/killbill/billing/account/ddl.sql | 3 + ...170123221645__add_lucky_search_indexes.sql | 3 + .../billing/account/AccountTestUtils.java | 8 +-- .../api/user/TestDefaultAccountUserApi.java | 58 ++++++++++++++++++ 7 files changed, 153 insertions(+), 5 deletions(-) create mode 100644 account/src/main/resources/org/killbill/billing/account/migration/V20170123221645__add_lucky_search_indexes.sql diff --git a/account/src/main/java/org/killbill/billing/account/dao/AccountSqlDao.java b/account/src/main/java/org/killbill/billing/account/dao/AccountSqlDao.java index d4747479cf..2eccd3cec6 100644 --- a/account/src/main/java/org/killbill/billing/account/dao/AccountSqlDao.java +++ b/account/src/main/java/org/killbill/billing/account/dao/AccountSqlDao.java @@ -60,4 +60,9 @@ public void updatePaymentMethod(@Bind("id") String accountId, @SqlQuery List getAccountsByParentId(@Bind("parentAccountId") UUID parentAccountId, @BindBean final InternalTenantContext context); + + @SqlQuery + public AccountModelDao luckySearch(@Bind("searchKey") final String searchKey, + @BindBean final InternalTenantContext context); + } diff --git a/account/src/main/java/org/killbill/billing/account/dao/DefaultAccountDao.java b/account/src/main/java/org/killbill/billing/account/dao/DefaultAccountDao.java index f1186a2a52..b959cb73c4 100644 --- a/account/src/main/java/org/killbill/billing/account/dao/DefaultAccountDao.java +++ b/account/src/main/java/org/killbill/billing/account/dao/DefaultAccountDao.java @@ -38,6 +38,7 @@ import org.killbill.billing.util.cache.CacheControllerDispatcher; import org.killbill.billing.util.callcontext.InternalCallContextFactory; import org.killbill.billing.util.dao.NonEntityDao; +import org.killbill.billing.util.entity.DefaultPagination; import org.killbill.billing.util.entity.Pagination; import org.killbill.billing.util.entity.dao.DefaultPaginationSqlDaoHelper.PaginationIteratorBuilder; import org.killbill.billing.util.entity.dao.EntityDaoBase; @@ -51,6 +52,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import com.google.common.collect.ImmutableList; import com.google.inject.Inject; public class DefaultAccountDao extends EntityDaoBase implements AccountDao { @@ -110,6 +112,25 @@ public AccountModelDao inTransaction(final EntitySqlDaoWrapperFactory entitySqlD @Override public Pagination searchAccounts(final String searchKey, final Long offset, final Long limit, final InternalTenantContext context) { + final boolean userIsFeelingLucky = limit == 1 && offset == -1; + if (userIsFeelingLucky) { + // The use-case we can optimize is when the user is looking for an exact match (e.g. he knows the full email). In that case, we can speed up the queries + // by doing exact searches only. + final AccountModelDao accountModelDao = transactionalSqlDao.execute(new EntitySqlDaoTransactionWrapper() { + @Override + public AccountModelDao inTransaction(final EntitySqlDaoWrapperFactory entitySqlDaoWrapperFactory) throws Exception { + return entitySqlDaoWrapperFactory.become(AccountSqlDao.class).luckySearch(searchKey, context); + } + }); + return new DefaultPagination(0L, + 1L, + accountModelDao == null ? 0L : 1L, + null, // We don't compute stats for speed in that case + accountModelDao == null ? ImmutableList.of().iterator() : ImmutableList.of(accountModelDao).iterator()); + } + + // Otherwise, we pretty much need to do a full table scan (leading % in the like clause). + // Note: forcing MySQL to search indexes (like luckySearch above) doesn't always seem to help on large tables, especially with large offsets return paginationHelper.getPagination(AccountSqlDao.class, new PaginationIteratorBuilder() { @Override diff --git a/account/src/main/resources/org/killbill/billing/account/dao/AccountSqlDao.sql.stg b/account/src/main/resources/org/killbill/billing/account/dao/AccountSqlDao.sql.stg index d48a16bb65..f9ddfa5dd1 100644 --- a/account/src/main/resources/org/killbill/billing/account/dao/AccountSqlDao.sql.stg +++ b/account/src/main/resources/org/killbill/billing/account/dao/AccountSqlDao.sql.stg @@ -106,6 +106,64 @@ searchQuery(prefix) ::= << or company_name like :likeSearchKey >> +/** Unfortunately, we need to force MySQL to use the individual indexes */ +luckySearch() ::= << +select + +from t +join ( + select distinct + from ( + ( + select + from + where = :searchKey + + + limit 1 + ) + union all + ( + select + from + where name = :searchKey + + + limit 1 + ) + union all + ( + select + from + where email = :searchKey + + + limit 1 + ) + union all + ( + select + from + where external_key = :searchKey + + + limit 1 + ) + union all + ( + select + from + where company_name = :searchKey + + + limit 1 + ) + ) search_with_idx +) optimization on = +limit 1 +; +>> + getIdFromKey() ::= << SELECT id FROM accounts @@ -119,4 +177,4 @@ getAccountsByParentId() ::= << ; ->> \ No newline at end of file +>> diff --git a/account/src/main/resources/org/killbill/billing/account/ddl.sql b/account/src/main/resources/org/killbill/billing/account/ddl.sql index a24f27235c..195eb5b428 100644 --- a/account/src/main/resources/org/killbill/billing/account/ddl.sql +++ b/account/src/main/resources/org/killbill/billing/account/ddl.sql @@ -37,6 +37,9 @@ CREATE UNIQUE INDEX accounts_id ON accounts(id); CREATE UNIQUE INDEX accounts_external_key ON accounts(external_key, tenant_record_id); CREATE INDEX accounts_parents ON accounts(parent_account_id); CREATE INDEX accounts_tenant_record_id ON accounts(tenant_record_id); +CREATE INDEX accounts_email_tenant_record_id ON accounts(email, tenant_record_id); +CREATE INDEX accounts_company_name_tenant_record_id ON accounts(company_name, tenant_record_id); +CREATE INDEX accounts_name_tenant_record_id ON accounts(name, tenant_record_id); DROP TABLE IF EXISTS account_history; CREATE TABLE account_history ( diff --git a/account/src/main/resources/org/killbill/billing/account/migration/V20170123221645__add_lucky_search_indexes.sql b/account/src/main/resources/org/killbill/billing/account/migration/V20170123221645__add_lucky_search_indexes.sql new file mode 100644 index 0000000000..54cd629925 --- /dev/null +++ b/account/src/main/resources/org/killbill/billing/account/migration/V20170123221645__add_lucky_search_indexes.sql @@ -0,0 +1,3 @@ +alter table accounts add index accounts_email_tenant_record_id(email, tenant_record_id); +alter table accounts add index accounts_company_name_tenant_record_id(company_name, tenant_record_id); +alter table accounts add index accounts_name_tenant_record_id(name, tenant_record_id); diff --git a/account/src/test/java/org/killbill/billing/account/AccountTestUtils.java b/account/src/test/java/org/killbill/billing/account/AccountTestUtils.java index 7934e126a2..8b532642a6 100644 --- a/account/src/test/java/org/killbill/billing/account/AccountTestUtils.java +++ b/account/src/test/java/org/killbill/billing/account/AccountTestUtils.java @@ -20,12 +20,12 @@ import java.util.UUID; import org.joda.time.DateTimeZone; -import org.testng.Assert; - import org.killbill.billing.account.api.AccountData; import org.killbill.billing.account.api.DefaultMutableAccountData; +import org.killbill.billing.account.api.MutableAccountData; import org.killbill.billing.account.dao.AccountModelDao; import org.killbill.billing.catalog.api.Currency; +import org.testng.Assert; public abstract class AccountTestUtils { @@ -81,11 +81,11 @@ private static AccountModelDao createTestAccount(final int billCycleDayUTC, fina return new AccountModelDao(UUID.randomUUID(), accountData); } - public static AccountData createAccountData() { + public static MutableAccountData createAccountData() { return createAccountData(30, 31, UUID.randomUUID().toString().substring(0, 4)); } - private static AccountData createAccountData(final int billCycleDayUTC, final int billCycleDayLocal, final String phone) { + private static MutableAccountData createAccountData(final int billCycleDayUTC, final int billCycleDayLocal, final String phone) { final String externalKey = UUID.randomUUID().toString(); final String email = UUID.randomUUID().toString().substring(0, 4) + '@' + UUID.randomUUID().toString().substring(0, 4); final String name = UUID.randomUUID().toString(); diff --git a/account/src/test/java/org/killbill/billing/account/api/user/TestDefaultAccountUserApi.java b/account/src/test/java/org/killbill/billing/account/api/user/TestDefaultAccountUserApi.java index 338c58cbe7..6149547a2f 100644 --- a/account/src/test/java/org/killbill/billing/account/api/user/TestDefaultAccountUserApi.java +++ b/account/src/test/java/org/killbill/billing/account/api/user/TestDefaultAccountUserApi.java @@ -34,18 +34,76 @@ import org.killbill.billing.account.dao.AccountModelDao; import org.killbill.billing.catalog.api.Currency; import org.killbill.billing.events.AccountCreationInternalEvent; +import org.killbill.billing.util.entity.Pagination; import org.testng.Assert; import org.testng.annotations.Test; +import com.google.common.collect.ImmutableList; import com.google.common.eventbus.Subscribe; import static com.jayway.awaitility.Awaitility.await; import static java.util.concurrent.TimeUnit.SECONDS; +import static org.killbill.billing.account.AccountTestUtils.createAccountData; import static org.killbill.billing.account.AccountTestUtils.createTestAccount; import static org.testng.Assert.assertEquals; public class TestDefaultAccountUserApi extends AccountTestSuiteWithEmbeddedDB { + @Test(groups = "slow", description = "Test Account search") + public void testSearch() throws Exception { + final MutableAccountData mutableAccountData1 = createAccountData(); + mutableAccountData1.setEmail("john@acme.com"); + mutableAccountData1.setCompanyName("Acme, Inc."); + final AccountModelDao account1ModelDao = new AccountModelDao(UUID.randomUUID(), mutableAccountData1); + final AccountData accountData1 = new DefaultAccount(account1ModelDao); + accountUserApi.createAccount(accountData1, callContext); + + final MutableAccountData mutableAccountData2 = createAccountData(); + mutableAccountData2.setEmail("bob@gmail.com"); + mutableAccountData2.setCompanyName("Acme, Inc."); + final AccountModelDao account2ModelDao = new AccountModelDao(UUID.randomUUID(), mutableAccountData2); + final AccountData accountData2 = new DefaultAccount(account2ModelDao); + accountUserApi.createAccount(accountData2, callContext); + + final Pagination search1 = accountUserApi.searchAccounts("Inc.", 0L, 5L, callContext); + Assert.assertEquals(search1.getCurrentOffset(), (Long) 0L); + Assert.assertNull(search1.getNextOffset()); + Assert.assertEquals(search1.getMaxNbRecords(), (Long) 2L); + Assert.assertEquals(search1.getTotalNbRecords(), (Long) 2L); + Assert.assertEquals(ImmutableList.copyOf(search1.iterator()).size(), 2); + + final Pagination search2 = accountUserApi.searchAccounts("Inc.", 0L, 1L, callContext); + Assert.assertEquals(search2.getCurrentOffset(), (Long) 0L); + Assert.assertEquals(search2.getNextOffset(), (Long) 1L); + Assert.assertEquals(search2.getMaxNbRecords(), (Long) 2L); + Assert.assertEquals(search2.getTotalNbRecords(), (Long) 2L); + Assert.assertEquals(ImmutableList.copyOf(search2.iterator()).size(), 1); + + final Pagination search3 = accountUserApi.searchAccounts("acme.com", 0L, 5L, callContext); + Assert.assertEquals(search3.getCurrentOffset(), (Long) 0L); + Assert.assertNull(search3.getNextOffset()); + Assert.assertEquals(search3.getMaxNbRecords(), (Long) 2L); + Assert.assertEquals(search3.getTotalNbRecords(), (Long) 1L); + Assert.assertEquals(ImmutableList.copyOf(search3.iterator()).size(), 1); + + // Exact search will fail + final Pagination search4 = accountUserApi.searchAccounts("acme.com", -1L, 1L, callContext); + Assert.assertEquals(search4.getCurrentOffset(), (Long) 0L); + Assert.assertNull(search4.getNextOffset()); + // Not computed + Assert.assertNull(search4.getMaxNbRecords()); + Assert.assertEquals(search4.getTotalNbRecords(), (Long) 0L); + Assert.assertEquals(ImmutableList.copyOf(search4.iterator()).size(), 0); + + final Pagination search5 = accountUserApi.searchAccounts("john@acme.com", -1L, 1L, callContext); + Assert.assertEquals(search5.getCurrentOffset(), (Long) 0L); + Assert.assertNull(search5.getNextOffset()); + // Not computed + Assert.assertNull(search5.getMaxNbRecords()); + Assert.assertEquals(search5.getTotalNbRecords(), (Long) 1L); + Assert.assertEquals(ImmutableList.copyOf(search5.iterator()).size(), 1); + } + @Test(groups = "slow", description = "Test Account creation generates an event") public void testBusEvents() throws Exception { final AccountEventHandler eventHandler = new AccountEventHandler(); From da0fa7806bc15fb3b8b53ec3a0dd042a7c7b16f3 Mon Sep 17 00:00:00 2001 From: Pierre-Alexandre Meyer Date: Mon, 23 Jan 2017 23:56:32 -0800 Subject: [PATCH 2/6] travis: tweak logging We're currently hitting the maximum time limit for jobs (50'). Let's see if reducing log output helps speed them up (less I/O?), otherwise we may have to disable some tests. Signed-off-by: Pierre-Alexandre Meyer --- profiles/killbill/src/test/resources/logback.travis.xml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/profiles/killbill/src/test/resources/logback.travis.xml b/profiles/killbill/src/test/resources/logback.travis.xml index 32029744e1..c8380184fe 100644 --- a/profiles/killbill/src/test/resources/logback.travis.xml +++ b/profiles/killbill/src/test/resources/logback.travis.xml @@ -29,6 +29,12 @@ + + + + + + From 3dc02277c78d6f9822a75ef9e30e3a986b409d6a Mon Sep 17 00:00:00 2001 From: Pierre-Alexandre Meyer Date: Tue, 24 Jan 2017 09:21:17 -0800 Subject: [PATCH 3/6] travis: disable beatrix tests This seems the most straightforward way to cut down our build time. It's also a work-around for flaky tests (see https://github.com/killbill/killbill/issues/485). Signed-off-by: Pierre-Alexandre Meyer --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index e8847be803..fc59cb008b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -6,7 +6,7 @@ sudo: false # directories: # - $HOME/.m2 -script: if [[ -v COMMAND ]]; then $COMMAND; else travis_retry mvn -q -Djava.security.egd=file:/dev/./urandom -Dorg.slf4j.simpleLogger.defaultLogLevel=WARN -Dorg.slf4j.simpleLogger.log.org.killbill.billing.util.cache=ERROR -Dorg.slf4j.simpleLogger.log.org.killbill.billing.lifecycle=ERROR -Dlogback.configurationFile=$PWD/profiles/killbill/src/test/resources/logback.travis.xml clean install $PHASE 2>&1 | egrep -v 'Download|Install|[ \t]*at [ \ta-zA-Z0-9\.\:\(\)]+'; [ ${PIPESTATUS[0]} == 0 ]; fi +script: if [[ -v COMMAND ]]; then $COMMAND; else travis_retry mvn -q -Djava.security.egd=file:/dev/./urandom -Dorg.slf4j.simpleLogger.defaultLogLevel=WARN -Dorg.slf4j.simpleLogger.log.org.killbill.billing.util.cache=ERROR -Dorg.slf4j.simpleLogger.log.org.killbill.billing.lifecycle=ERROR -Dlogback.configurationFile=$PWD/profiles/killbill/src/test/resources/logback.travis.xml clean install $PHASE -pl '!beatrix' 2>&1 | egrep -v 'Download|Install|[ \t]*at [ \ta-zA-Z0-9\.\:\(\)]+'; [ ${PIPESTATUS[0]} == 0 ]; fi # Remove --quiet to avoid timeouts install: mvn -U install -DskipTests=true | egrep -v 'Download|Install' From 9c691d59dc92151614529306932260b2bede2eef Mon Sep 17 00:00:00 2001 From: Pierre-Alexandre Meyer Date: Tue, 24 Jan 2017 10:17:27 -0800 Subject: [PATCH 4/6] payment: trivial cleanup Signed-off-by: Pierre-Alexandre Meyer --- .../billing/payment/core/sm/PaymentStateMachineHelper.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/payment/src/main/java/org/killbill/billing/payment/core/sm/PaymentStateMachineHelper.java b/payment/src/main/java/org/killbill/billing/payment/core/sm/PaymentStateMachineHelper.java index 7e010b7489..daee9eb96e 100644 --- a/payment/src/main/java/org/killbill/billing/payment/core/sm/PaymentStateMachineHelper.java +++ b/payment/src/main/java/org/killbill/billing/payment/core/sm/PaymentStateMachineHelper.java @@ -68,7 +68,7 @@ public class PaymentStateMachineHelper { private static final String VOID_FAILED = "VOID_FAILED"; private static final String CHARGEBACK_FAILED = "CHARGEBACK_FAILED"; - private static final String AUTH_ERRORED = "AUTH_ERRORED"; + private static final String AUTHORIZE_ERRORED = "AUTH_ERRORED"; private static final String CAPTURE_ERRORED = "CAPTURE_ERRORED"; private static final String PURCHASE_ERRORED = "PURCHASE_ERRORED"; private static final String REFUND_ERRORED = "REFUND_ERRORED"; @@ -78,7 +78,7 @@ public class PaymentStateMachineHelper { private final StateMachineConfigCache stateMachineConfigCache; - public static final String[] STATE_NAMES = {AUTH_ERRORED, + public static final String[] STATE_NAMES = {AUTHORIZE_ERRORED, AUTHORIZE_FAILED, AUTHORIZE_PENDING, AUTHORIZE_SUCCESS, @@ -161,7 +161,7 @@ public String getPendingStateForTransaction(final TransactionType transactionTyp public String getErroredStateForTransaction(final TransactionType transactionType) { switch (transactionType) { case AUTHORIZE: - return AUTH_ERRORED; + return AUTHORIZE_ERRORED; case CAPTURE: return CAPTURE_ERRORED; case PURCHASE: From aff04b8eac4def309d5a4f35855eacbc74be6abb Mon Sep 17 00:00:00 2001 From: Pierre-Alexandre Meyer Date: Tue, 24 Jan 2017 10:19:59 -0800 Subject: [PATCH 5/6] payment: trivial renaming in DefaultPaymentDao Signed-off-by: Pierre-Alexandre Meyer --- .../org/killbill/billing/payment/dao/DefaultPaymentDao.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/payment/src/main/java/org/killbill/billing/payment/dao/DefaultPaymentDao.java b/payment/src/main/java/org/killbill/billing/payment/dao/DefaultPaymentDao.java index bed7b5bf9d..020ff4974b 100644 --- a/payment/src/main/java/org/killbill/billing/payment/dao/DefaultPaymentDao.java +++ b/payment/src/main/java/org/killbill/billing/payment/dao/DefaultPaymentDao.java @@ -254,7 +254,7 @@ public Iterator build(final PaymentSqlDao paymentSqlDao, final @Override public Pagination searchPayments(final String searchKey, final Long offset, final Long limit, final InternalTenantContext context) { // Optimization: if the search key looks like a state name (e.g. _ERRORED), assume the user is searching by state only - final List paymentStates = shouldSearchByStateNameOnly(searchKey); + final List paymentStates = expandSearchFilterToStateNames(searchKey); final String likeSearchKey = String.format("%%%s%%", searchKey); return paginationHelper.getPagination(PaymentSqlDao.class, @@ -274,7 +274,7 @@ public Iterator build(final PaymentSqlDao paymentSqlDao, final context); } - private List shouldSearchByStateNameOnly(final String searchKey) { + private List expandSearchFilterToStateNames(final String searchKey) { final Pattern pattern = Pattern.compile(".*" + searchKey + ".*"); // Note that technically, we should look at all of the available state names in the database instead since the state machine is configurable. The common use-case From e01070ccd46df4fe274b9b79f73793ca4dda184e Mon Sep 17 00:00:00 2001 From: Pierre-Alexandre Meyer Date: Tue, 24 Jan 2017 10:35:09 -0800 Subject: [PATCH 6/6] travis: disable server tests Unfortunately, disabling beatrix tests wasn't enough. Signed-off-by: Pierre-Alexandre Meyer --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index fc59cb008b..531da15473 100644 --- a/.travis.yml +++ b/.travis.yml @@ -6,7 +6,7 @@ sudo: false # directories: # - $HOME/.m2 -script: if [[ -v COMMAND ]]; then $COMMAND; else travis_retry mvn -q -Djava.security.egd=file:/dev/./urandom -Dorg.slf4j.simpleLogger.defaultLogLevel=WARN -Dorg.slf4j.simpleLogger.log.org.killbill.billing.util.cache=ERROR -Dorg.slf4j.simpleLogger.log.org.killbill.billing.lifecycle=ERROR -Dlogback.configurationFile=$PWD/profiles/killbill/src/test/resources/logback.travis.xml clean install $PHASE -pl '!beatrix' 2>&1 | egrep -v 'Download|Install|[ \t]*at [ \ta-zA-Z0-9\.\:\(\)]+'; [ ${PIPESTATUS[0]} == 0 ]; fi +script: if [[ -v COMMAND ]]; then $COMMAND; else travis_retry mvn -q -Djava.security.egd=file:/dev/./urandom -Dorg.slf4j.simpleLogger.defaultLogLevel=WARN -Dorg.slf4j.simpleLogger.log.org.killbill.billing.util.cache=ERROR -Dorg.slf4j.simpleLogger.log.org.killbill.billing.lifecycle=ERROR -Dlogback.configurationFile=$PWD/profiles/killbill/src/test/resources/logback.travis.xml clean install $PHASE -pl '!beatrix,!profiles,!profiles/killbill,!profiles/killpay' 2>&1 | egrep -v 'Download|Install|[ \t]*at [ \ta-zA-Z0-9\.\:\(\)]+'; [ ${PIPESTATUS[0]} == 0 ]; fi # Remove --quiet to avoid timeouts install: mvn -U install -DskipTests=true | egrep -v 'Download|Install'