[GitHub] incubator-omid pull request #23: [OMID-89] Fix metrics in Persistence proces...

2018-02-13 Thread ohadshacham 
Github user ohadshacham commented on a diff in the pull request:

https://github.com/apache/incubator-omid/pull/23#discussion_r167857492
  
--- Diff: 
tso-server/src/test/java/org/apache/omid/tso/TestPersistenceProcessorHandler.java
 ---
@@ -126,6 +126,36 @@ void afterMethod() {
 Mockito.reset(mockWriter);
 }
 
+@Test(timeOut = 1_000)
+public void testPersistentProcessorHandlerIdsAreCreatedConsecutive() 
throws Exception {
+
+TSOServerConfig tsoConfig = new TSOServerConfig();
+tsoConfig.setNumConcurrentCTWriters(32);
+
+PersistenceProcessorHandler[] handlers = new 
PersistenceProcessorHandler[tsoConfig.getNumConcurrentCTWriters()];
+for (int i = 0; i < tsoConfig.getNumConcurrentCTWriters(); i++) {
+handlers[i] = new PersistenceProcessorHandler(metrics,
+  "localhost:1234",
+  
mock(LeaseManager.class),
+  commitTable,
+  
mock(ReplyProcessor.class),
+  retryProcessor,
+  panicker);
+}
+
+for (int i = 0; i < tsoConfig.getNumConcurrentCTWriters(); i++) {
+// Required to generalize the cases when other tests have 
increased the static variable assigning the ids
+if (i + 1 < tsoConfig.getNumConcurrentCTWriters()) {
+int followingHandlerIdAsInt = Integer.valueOf(handlers[i + 
1].getId());
+assertEquals(handlers[i].getId(), 
String.valueOf(followingHandlerIdAsInt - 1));
--- End diff --

We kind of testing the atomic integer :)


---

[jira] [Commented] (OMID-87) Fix BatchPool initialization

2018-02-13 Thread Ohad Shacham (JIRA) 

[ 
https://issues.apache.org/jira/browse/OMID-87?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16362301#comment-16362301
 ] 

Ohad Shacham commented on OMID-87:
--

LGTM

> Fix BatchPool initialization
> 
>
> Key: OMID-87
> URL: https://issues.apache.org/jira/browse/OMID-87
> Project: Apache Omid
>  Issue Type: Bug
>Affects Versions: 0.8.2.0
>Reporter: Francisco Perez-Sorrosal
>Assignee: Francisco Perez-Sorrosal
>Priority: Critical
>  Labels: batch, batchpool, creation, fix
> Fix For: 0.9.0.0
>
>
> When initializing the ObjectPool for Batch java objects in 
> BatchPoolModule.java, an Apache's GenericObjectPool is used. The pool 
> configuration lacks the call to config.setMaxIdle(int), which provokes that 
> when the Batch objects are returned to the ObjectPool during the pre-creation 
> phase, those above the default max idle number of objects in the pool (which 
> is 8 according to the [Apache 
> documentation|https://commons.apache.org/proper/commons-pool/api-1.6/org/apache/commons/pool/impl/GenericObjectPool.html])
>  are destroyed.
> If at some point the TSO needs to access those Batch objects between 8 and 
> the number specified in the Omid configuration (numConcurrentCTWriters), they 
> will be created on the fly, which it was we want to avoid when pre-creating 
> the ObjectPool during the TSO initialization.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (OMID-89) Fix metrics in PersistenceProcesssorHandlers

2018-02-13 Thread ASF GitHub Bot (JIRA) 

[ 
https://issues.apache.org/jira/browse/OMID-89?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16362275#comment-16362275
 ] 

ASF GitHub Bot commented on OMID-89:


Github user ohadshacham commented on a diff in the pull request:

https://github.com/apache/incubator-omid/pull/23#discussion_r167857492
  
--- Diff: 
tso-server/src/test/java/org/apache/omid/tso/TestPersistenceProcessorHandler.java
 ---
@@ -126,6 +126,36 @@ void afterMethod() {
 Mockito.reset(mockWriter);
 }
 
+@Test(timeOut = 1_000)
+public void testPersistentProcessorHandlerIdsAreCreatedConsecutive() 
throws Exception {
+
+TSOServerConfig tsoConfig = new TSOServerConfig();
+tsoConfig.setNumConcurrentCTWriters(32);
+
+PersistenceProcessorHandler[] handlers = new 
PersistenceProcessorHandler[tsoConfig.getNumConcurrentCTWriters()];
+for (int i = 0; i < tsoConfig.getNumConcurrentCTWriters(); i++) {
+handlers[i] = new PersistenceProcessorHandler(metrics,
+  "localhost:1234",
+  
mock(LeaseManager.class),
+  commitTable,
+  
mock(ReplyProcessor.class),
+  retryProcessor,
+  panicker);
+}
+
+for (int i = 0; i < tsoConfig.getNumConcurrentCTWriters(); i++) {
+// Required to generalize the cases when other tests have 
increased the static variable assigning the ids
+if (i + 1 < tsoConfig.getNumConcurrentCTWriters()) {
+int followingHandlerIdAsInt = Integer.valueOf(handlers[i + 
1].getId());
+assertEquals(handlers[i].getId(), 
String.valueOf(followingHandlerIdAsInt - 1));
--- End diff --

We kind of testing the atomic integer :)


> Fix metrics in PersistenceProcesssorHandlers
> 
>
> Key: OMID-89
> URL: https://issues.apache.org/jira/browse/OMID-89
> Project: Apache Omid
>  Issue Type: Bug
>Affects Versions: 0.8.2.0
>Reporter: Francisco Perez-Sorrosal
>Assignee: Francisco Perez-Sorrosal
>Priority: Minor
>  Labels: handler, metrics, persistence, processor
> Fix For: 0.9.0.0
>
>
> Currently all the processor handlers report to the same metrics output. 
> Separate the metrics of each handler to let each of them report individually 
> through a handler identifier.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (OMID-86) Add Tx info in RollbackException

2018-02-13 Thread Ohad Shacham (JIRA) 

[ 
https://issues.apache.org/jira/browse/OMID-86?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16362229#comment-16362229
 ] 

Ohad Shacham commented on OMID-86:
--

LGTM +1

> Add Tx info in RollbackException
> 
>
> Key: OMID-86
> URL: https://issues.apache.org/jira/browse/OMID-86
> Project: Apache Omid
>  Issue Type: Improvement
>Affects Versions: 0.8.2.0
>Reporter: Francisco Perez-Sorrosal
>Assignee: Francisco Perez-Sorrosal
>Priority: Trivial
> Fix For: 0.9.0.0
>
>
> Add missing transaction information in all RollbackException in 
> AbstractTransactionManager.
> Required for debugging purposes.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)