This is an automated email from the ASF dual-hosted git repository.
jchen21 pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push:
new cbfa6a8 GEODE-6511 Change data source to use connection pool by
default (#3293)
cbfa6a8 is described below
commit cbfa6a88f700ddd6340b54029d215eaa0b65e128
Author: BenjaminPerryRoss <[email protected]>
AuthorDate: Wed Mar 13 12:11:51 2019 -0700
GEODE-6511 Change data source to use connection pool by default (#3293)
Co-authored-by: Ben Ross <[email protected]>
Co-authored-by: Jianxia Chen <[email protected]>
---
.../cli/DescribeDataSourceCommandDUnitTest.java | 16 ++--
.../cli/DestroyDataSourceCommandDUnitTest.java | 88 +++++++++++++++++++++-
.../cli/ListDataSourceCommandDUnitTest.java | 18 ++++-
.../jdbc/internal/cli/CreateDataSourceCommand.java | 9 ++-
.../functions/DestroyJndiBindingFunctionTest.java | 2 +-
.../apache/geode/internal/jndi/JNDIInvoker.java | 14 ++--
.../cli/functions/DestroyJndiBindingFunction.java | 6 +-
7 files changed, 128 insertions(+), 25 deletions(-)
diff --git
a/geode-connectors/src/distributedTest/java/org/apache/geode/connectors/jdbc/internal/cli/DescribeDataSourceCommandDUnitTest.java
b/geode-connectors/src/distributedTest/java/org/apache/geode/connectors/jdbc/internal/cli/DescribeDataSourceCommandDUnitTest.java
index 4b08344..cdb4bfb 100644
---
a/geode-connectors/src/distributedTest/java/org/apache/geode/connectors/jdbc/internal/cli/DescribeDataSourceCommandDUnitTest.java
+++
b/geode-connectors/src/distributedTest/java/org/apache/geode/connectors/jdbc/internal/cli/DescribeDataSourceCommandDUnitTest.java
@@ -61,7 +61,7 @@ public class DescribeDataSourceCommandDUnitTest {
@Test
public void describeDataSourceForSimpleDataSource() {
gfsh.executeAndAssertThat(
- "create data-source --name=simple
--url=\"jdbc:derby:memory:newDB;create=true\" --username=joe
--password=myPassword")
+ "create data-source --name=simple --pooled=false
--url=\"jdbc:derby:memory:newDB;create=true\" --username=joe
--password=myPassword")
.statusIsSuccess().tableHasColumnOnlyWithValues("Member", "server-1");
CommandResultAssert result = gfsh.executeAndAssertThat("describe
data-source --name=simple");
@@ -87,7 +87,7 @@ public class DescribeDataSourceCommandDUnitTest {
private void executeSql(String sql) {
server.invoke(() -> {
try {
- DataSource ds = JNDIInvoker.getDataSource("simple");
+ DataSource ds = JNDIInvoker.getDataSource("pool");
Connection conn = ds.getConnection();
Statement sm = conn.createStatement();
sm.execute(sql);
@@ -136,24 +136,24 @@ public class DescribeDataSourceCommandDUnitTest {
@Test
public void describeDataSourceUsedByRegionsListsTheRegionsInOutput() {
gfsh.executeAndAssertThat(
- "create data-source --name=simple
--url=\"jdbc:derby:memory:newDB;create=true\"")
+ "create data-source --name=pool
--url=\"jdbc:derby:memory:newDB;create=true\"")
.statusIsSuccess().tableHasColumnOnlyWithValues("Member", "server-1");
gfsh.executeAndAssertThat("create region --name=region1
--type=REPLICATE").statusIsSuccess();
gfsh.executeAndAssertThat("create region --name=region2
--type=PARTITION").statusIsSuccess();
setupDatabase();
try {
gfsh.executeAndAssertThat(
- "create jdbc-mapping --region=region1 --data-source=simple
--pdx-name="
+ "create jdbc-mapping --region=region1 --data-source=pool --pdx-name="
+ IdAndName.class.getName() + " --schema=mySchema");
gfsh.executeAndAssertThat(
- "create jdbc-mapping --region=region2 --data-source=simple
--pdx-name="
+ "create jdbc-mapping --region=region2 --data-source=pool --pdx-name="
+ IdAndName.class.getName() + " --schema=mySchema");
- CommandResultAssert result = gfsh.executeAndAssertThat("describe
data-source --name=simple");
+ CommandResultAssert result = gfsh.executeAndAssertThat("describe
data-source --name=pool");
result.statusIsSuccess()
- .tableHasRowWithValues("Property", "Value", "name", "simple")
- .tableHasRowWithValues("Property", "Value", "pooled", "false")
+ .tableHasRowWithValues("Property", "Value", "name", "pool")
+ .tableHasRowWithValues("Property", "Value", "pooled", "true")
.tableHasRowWithValues("Property", "Value", "url",
"jdbc:derby:memory:newDB;create=true");
InfoResultModel infoSection = result.getResultModel()
.getInfoSection(DescribeDataSourceCommand.REGIONS_USING_DATA_SOURCE_SECTION);
diff --git
a/geode-connectors/src/distributedTest/java/org/apache/geode/connectors/jdbc/internal/cli/DestroyDataSourceCommandDUnitTest.java
b/geode-connectors/src/distributedTest/java/org/apache/geode/connectors/jdbc/internal/cli/DestroyDataSourceCommandDUnitTest.java
index 1df7163..c482405 100644
---
a/geode-connectors/src/distributedTest/java/org/apache/geode/connectors/jdbc/internal/cli/DestroyDataSourceCommandDUnitTest.java
+++
b/geode-connectors/src/distributedTest/java/org/apache/geode/connectors/jdbc/internal/cli/DestroyDataSourceCommandDUnitTest.java
@@ -72,7 +72,7 @@ public class DestroyDataSourceCommandDUnitTest {
.invokeInEveryMember(
() ->
assertThat(JNDIInvoker.getBindingNamesRecursively(JNDIInvoker.getJNDIContext()))
.containsKey("java:datasource1").containsValue(
-
"org.apache.geode.internal.datasource.GemFireBasicDataSource"),
+ "com.zaxxer.hikari.HikariDataSource"),
server1, server2);
gfsh.executeAndAssertThat("destroy data-source
--name=datasource1").statusIsSuccess()
@@ -120,6 +120,69 @@ public class DestroyDataSourceCommandDUnitTest {
});
}
+ @Test
+ public void testDestroySimpleDataSource() throws Exception {
+ // drop the default pooled data source
+ gfsh.executeAndAssertThat("destroy data-source
--name=datasource1").statusIsSuccess();
+
+ // create a simple data source, i.e. non-pooled data source
+ gfsh.execute(
+ "create data-source --name=datasource2
--url=\"jdbc:derby:memory:newDB;create=true\" --pooled=false");
+
+ // assert that there is a datasource
+ VMProvider
+ .invokeInEveryMember(
+ () ->
assertThat(JNDIInvoker.getBindingNamesRecursively(JNDIInvoker.getJNDIContext()))
+ .containsKey("java:datasource2").containsValue(
+
"org.apache.geode.internal.datasource.GemFireBasicDataSource"),
+ server1, server2);
+
+
+ gfsh.executeAndAssertThat("destroy data-source
--name=datasource2").statusIsSuccess()
+ .tableHasColumnOnlyWithValues("Member", "server-1", "server-2")
+ .tableHasColumnOnlyWithValues("Message",
+ "Data source \"datasource2\" destroyed on \"server-1\"",
+ "Data source \"datasource2\" destroyed on \"server-2\"");
+
+ // verify cluster config is updated
+ locator.invoke(() -> {
+ InternalLocator internalLocator = ClusterStartupRule.getLocator();
+ AssertionsForClassTypes.assertThat(internalLocator).isNotNull();
+ InternalConfigurationPersistenceService ccService =
+ internalLocator.getConfigurationPersistenceService();
+ Configuration configuration = ccService.getConfiguration("cluster");
+ Document document =
XmlUtils.createDocumentFromXml(configuration.getCacheXmlContent());
+ NodeList jndiBindings = document.getElementsByTagName("jndi-binding");
+
+
AssertionsForClassTypes.assertThat(jndiBindings.getLength()).isEqualTo(0);
+
+ boolean found = false;
+ for (int i = 0; i < jndiBindings.getLength(); i++) {
+ Element eachBinding = (Element) jndiBindings.item(i);
+ if (eachBinding.getAttribute("jndi-name").equals("datasource2")) {
+ found = true;
+ break;
+ }
+ }
+ AssertionsForClassTypes.assertThat(found).isFalse();
+ });
+
+ // verify datasource does not exists
+ VMProvider.invokeInEveryMember(
+ () ->
AssertionsForClassTypes.assertThat(JNDIInvoker.getNoOfAvailableDataSources())
+ .isEqualTo(0),
+ server1, server2);
+
+ // bounce server1 and assert that there is still no datasource received
from cluster config
+ server1.stop(false);
+ server1 = cluster.startServerVM(1, locator.getPort());
+
+ // verify no datasource from cluster config
+ server1.invoke(() -> {
+
AssertionsForClassTypes.assertThat(JNDIInvoker.getNoOfAvailableDataSources()).isEqualTo(0);
+ });
+ }
+
private void createTable() {
executeSql("create table mySchema.myRegion (id varchar(10) primary key,
name varchar(10))");
}
@@ -196,4 +259,27 @@ public class DestroyDataSourceCommandDUnitTest {
dropTable();
}
}
+
+ @Test
+ public void destroySimpleDataSourceFailsIfInUseByJdbcMapping() throws
Exception {
+ // create a simple data source, i.e. non-pooled data source
+ gfsh.execute(
+ "create data-source --name=datasource2
--url=\"jdbc:derby:memory:newDB;create=true\" --pooled=false");
+
+ gfsh.executeAndAssertThat("create region --name=myRegion
--type=REPLICATE").statusIsSuccess();
+ createTable();
+ try {
+ gfsh.executeAndAssertThat(
+ "create jdbc-mapping --data-source=datasource2 --pdx-name=" +
IdAndName.class.getName()
+ + " --region=myRegion --schema=mySchema")
+ .statusIsSuccess();
+
+ gfsh.executeAndAssertThat("destroy data-source
--name=datasource2").statusIsError()
+ .containsOutput(
+ "Data source named \"datasource2\" is still being used by region
\"myRegion\"."
+ + " Use destroy jdbc-mapping --region=myRegion and then try
again.");
+ } finally {
+ dropTable();
+ }
+ }
}
diff --git
a/geode-connectors/src/distributedTest/java/org/apache/geode/connectors/jdbc/internal/cli/ListDataSourceCommandDUnitTest.java
b/geode-connectors/src/distributedTest/java/org/apache/geode/connectors/jdbc/internal/cli/ListDataSourceCommandDUnitTest.java
index b334ee2..ca3df16 100644
---
a/geode-connectors/src/distributedTest/java/org/apache/geode/connectors/jdbc/internal/cli/ListDataSourceCommandDUnitTest.java
+++
b/geode-connectors/src/distributedTest/java/org/apache/geode/connectors/jdbc/internal/cli/ListDataSourceCommandDUnitTest.java
@@ -56,7 +56,7 @@ public class ListDataSourceCommandDUnitTest {
@Test
public void listDataSourceForSimpleDataSource() {
gfsh.executeAndAssertThat(
- "create data-source --name=simple
--url=\"jdbc:derby:memory:newDB;create=true\" --username=joe
--password=myPassword")
+ "create data-source --name=simple
--url=\"jdbc:derby:memory:newDB;create=true\" --username=joe
--password=myPassword --pooled=false ")
.statusIsSuccess().tableHasColumnOnlyWithValues("Member", "server-1");
CommandResultAssert result = gfsh.executeAndAssertThat("list data-source");
@@ -128,7 +128,7 @@ public class ListDataSourceCommandDUnitTest {
@Test
public void listDataSourceUsedByRegionsHasCorrectOutput() {
gfsh.executeAndAssertThat(
- "create data-source --name=simple
--url=\"jdbc:derby:memory:newDB;create=true\"")
+ "create data-source --name=simple
--url=\"jdbc:derby:memory:newDB;create=true\" --pooled=false")
.statusIsSuccess().tableHasColumnOnlyWithValues("Member", "server-1");
setupDatabase();
gfsh.executeAndAssertThat("create region --name=region1
--type=REPLICATE").statusIsSuccess();
@@ -163,9 +163,21 @@ public class ListDataSourceCommandDUnitTest {
}
@Test
+ public void listDataSourceForPooledDataSourceByDefault() {
+ gfsh.executeAndAssertThat(
+ "create data-source --name=pooledDataSource
--url=\"jdbc:derby:memory:newDB;create=true\"
--pooled-data-source-factory-class=org.apache.geode.internal.jta.CacheJTAPooledDataSourceFactory
--pool-properties={'name':'prop1','value':'value1'},{'name':'pool.prop2','value':'value2'}")
+ .statusIsSuccess().tableHasColumnOnlyWithValues("Member", "server-1");
+
+ gfsh.executeAndAssertThat("list data-source").statusIsSuccess()
+ .tableHasRowWithValues("name", "pooled", "in use", "url",
"pooledDataSource", "true",
+ "false",
+ "jdbc:derby:memory:newDB;create=true");
+ }
+
+ @Test
public void listDataSourceWithMultipleDataSourcesListsAll() {
gfsh.executeAndAssertThat(
- "create data-source --name=simple
--url=\"jdbc:derby:memory:newDB;create=true\" --username=joe
--password=myPassword")
+ "create data-source --name=simple --pooled=false
--url=\"jdbc:derby:memory:newDB;create=true\" --username=joe
--password=myPassword")
.statusIsSuccess().tableHasColumnOnlyWithValues("Member", "server-1");
gfsh.executeAndAssertThat(
"create data-source --name=pooledDataSource --pooled
--url=\"jdbc:derby:memory:newDB;create=true\"
--pooled-data-source-factory-class=org.apache.geode.internal.jta.CacheJTAPooledDataSourceFactory
--pool-properties={'name':'prop1','value':'value1'},{'name':'pool.prop2','value':'value2'}")
diff --git
a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/CreateDataSourceCommand.java
b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/CreateDataSourceCommand.java
index ea68cd7..c4b1255 100644
---
a/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/CreateDataSourceCommand.java
+++
b/geode-connectors/src/main/java/org/apache/geode/connectors/jdbc/internal/cli/CreateDataSourceCommand.java
@@ -91,12 +91,17 @@ public class CreateDataSourceCommand extends
SingleGfshCommand {
@CliOption(key = CliStrings.IFNOTEXISTS, help = IFNOTEXISTS__HELP,
specifiedDefaultValue = "true", unspecifiedDefaultValue = "false")
boolean ifNotExists,
@CliOption(key = POOLED, help = POOLED__HELP,
- specifiedDefaultValue = "true", unspecifiedDefaultValue = "false")
boolean pooled,
+ specifiedDefaultValue = "true", unspecifiedDefaultValue = "true")
boolean pooled,
@CliOption(key = POOL_PROPERTIES, optionContext =
"splittingRegex=,(?![^{]*\\})",
help = POOL_PROPERTIES_HELP) PoolProperty[] poolProperties) {
JndiBindingsType.JndiBinding configuration = new
JndiBindingsType.JndiBinding();
- configuration.setConnPooledDatasourceClass(pooledDataSourceFactoryClass);
+ if (pooled) {
+ if (pooledDataSourceFactoryClass == null) {
+ pooledDataSourceFactoryClass =
DEFAULT_POOLED_DATA_SOURCE_FACTORY_CLASS;
+ }
+ configuration.setConnPooledDatasourceClass(pooledDataSourceFactoryClass);
+ }
configuration.setConnectionUrl(url);
configuration.setJndiName(name);
configuration.setPassword(password);
diff --git
a/geode-core/src/integrationTest/java/org/apache/geode/management/internal/cli/functions/DestroyJndiBindingFunctionTest.java
b/geode-core/src/integrationTest/java/org/apache/geode/management/internal/cli/functions/DestroyJndiBindingFunctionTest.java
index 3717fdc..87758b4 100644
---
a/geode-core/src/integrationTest/java/org/apache/geode/management/internal/cli/functions/DestroyJndiBindingFunctionTest.java
+++
b/geode-core/src/integrationTest/java/org/apache/geode/management/internal/cli/functions/DestroyJndiBindingFunctionTest.java
@@ -134,7 +134,7 @@ public class DestroyJndiBindingFunctionTest {
when(context.getArguments()).thenReturn(new Object[] {"jndi1", true});
when(context.getResultSender()).thenReturn(resultSender);
DestroyJndiBindingFunction destroyFunctionSpy =
spy(destroyJndiBindingFunction);
- doReturn(true).when(destroyFunctionSpy).checkForInvalidDataSource(any());
+ doReturn(false).when(destroyFunctionSpy).isValidDataSource(any());
destroyFunctionSpy.execute(context);
verify(resultSender).lastResult(resultCaptor.capture());
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/jndi/JNDIInvoker.java
b/geode-core/src/main/java/org/apache/geode/internal/jndi/JNDIInvoker.java
index 5c050d3..8380539 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/jndi/JNDIInvoker.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/jndi/JNDIInvoker.java
@@ -48,8 +48,7 @@ import
org.apache.geode.internal.datasource.ClientConnectionFactoryWrapper;
import org.apache.geode.internal.datasource.ConfigProperty;
import org.apache.geode.internal.datasource.DataSourceCreateException;
import org.apache.geode.internal.datasource.DataSourceFactory;
-import org.apache.geode.internal.datasource.GemFireBasicDataSource;
-import org.apache.geode.internal.datasource.GemFireConnPooledDataSource;
+import org.apache.geode.internal.datasource.GemFireTransactionDataSource;
import org.apache.geode.internal.jta.TransactionManagerImpl;
import org.apache.geode.internal.jta.TransactionUtils;
import org.apache.geode.internal.jta.UserTransactionImpl;
@@ -406,14 +405,15 @@ public class JNDIInvoker {
}
}
- public static boolean checkForInvalidDataSource(String name) {
+ public static boolean isValidDataSource(String name) {
Object dataSource = dataSourceMap.get(name);
- if (dataSource == null || dataSource instanceof GemFireBasicDataSource
- || dataSource instanceof GemFireConnPooledDataSource) {
- return false;
+ if (dataSource == null || (dataSource != null && dataSource instanceof
DataSource
+ && !(dataSource instanceof GemFireTransactionDataSource))) {
+ return true;
}
- return true;
+
+ return false;
}
/**
diff --git
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/DestroyJndiBindingFunction.java
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/DestroyJndiBindingFunction.java
index a31d8bd..eba763e 100644
---
a/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/DestroyJndiBindingFunction.java
+++
b/geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/DestroyJndiBindingFunction.java
@@ -32,7 +32,7 @@ public class DestroyJndiBindingFunction extends
CliFunction<Object[]> {
if (destroyingDataSource) {
typeName = "Data source";
- if (checkForInvalidDataSource(jndiName)) {
+ if (!isValidDataSource(jndiName)) {
return new CliFunctionResult(context.getMemberName(),
CliFunctionResult.StatusState.ERROR,
CliStrings.format(
"Data Source {0} has invalid type for destroy data-source,
destroy jndi-binding command should be used.",
@@ -53,7 +53,7 @@ public class DestroyJndiBindingFunction extends
CliFunction<Object[]> {
}
}
- boolean checkForInvalidDataSource(String jndiName) {
- return JNDIInvoker.checkForInvalidDataSource(jndiName);
+ boolean isValidDataSource(String jndiName) {
+ return JNDIInvoker.isValidDataSource(jndiName);
}
}
