date:20170227

2017-02-27 Thread ASF subversion and git services (JIRA)


[ 
https://issues.apache.org/jira/browse/GEODE-880?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15887361#comment-15887361
 ] 

ASF GitHub Bot commented on GEODE-880:
--

GitHub user metatype opened a pull request:

https://github.com/apache/geode/pull/409

GEODE-880 Remove unused classes

@kirklund I think this should close out GEODE-880.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/metatype/incubator-geode feature/GEODE-880

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/geode/pull/409.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #409


commit d00bfc4971d6e8e344dbe83debfdf8929a1b5bda
Author: Anthony Baker 
Date:   2017-02-28T06:04:46Z

GEODE-880 Remove unused classes




> Remove unused classes
> -
>
> Key: GEODE-880
> URL: https://issues.apache.org/jira/browse/GEODE-880
> Project: Geode
>  Issue Type: Wish
>  Components: general
>Reporter: Kirk Lund
>
> We would typically only keep unused classes if they were part of the user API 
> in a previous release.
> If an unused class is not required for backwards compatibility then it should 
> be deleted. This jira ticket lists unused classes that should be removed. If 
> you delete any of these classes, please add an appropriate comment and then 
> remove the class name from the ticket description.
> * geode-core/src/test/java/com/gemstone/gemfire/internal/JavaExec.java
> * geode-core/src/test/java/com/gemstone/gemfire/internal/LongBuffer.java



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] geode pull request #409: GEODE-880 Remove unused classes

2017-02-27 Thread metatype

GitHub user metatype opened a pull request:

https://github.com/apache/geode/pull/409

GEODE-880 Remove unused classes

@kirklund I think this should close out GEODE-880.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/metatype/incubator-geode feature/GEODE-880

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/geode/pull/409.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #409


commit d00bfc4971d6e8e344dbe83debfdf8929a1b5bda
Author: Anthony Baker 
Date:   2017-02-28T06:04:46Z

GEODE-880 Remove unused classes




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Updated] (GEODE-728) Rename Execution.withArgs to Execution.setArguments

2017-02-27 Thread Anthony Baker (JIRA)


 [ 
https://issues.apache.org/jira/browse/GEODE-728?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Anthony Baker updated GEODE-728:

Labels:   (was: starter)

> Rename Execution.withArgs to Execution.setArguments
> ---
>
> Key: GEODE-728
> URL: https://issues.apache.org/jira/browse/GEODE-728
> Project: Geode
>  Issue Type: Improvement
>  Components: docs, functions
>Reporter: Dan Smith
>Assignee: Alyssa Kim
>
> FunctionContext has a getArguments method. withArgs should be renamed to 
> match.
> See this discussion on the mailing list.
> http://mail-archives.apache.org/mod_mbox/incubator-geode-dev/201512.mbox/browser



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Updated] (GEODE-1577) Unhelpful generic types on Execution.execute

2017-02-27 Thread Anthony Baker (JIRA)


 [ 
https://issues.apache.org/jira/browse/GEODE-1577?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Anthony Baker updated GEODE-1577:
-
Labels:   (was: starter)

> Unhelpful generic types on Execution.execute
> 
>
> Key: GEODE-1577
> URL: https://issues.apache.org/jira/browse/GEODE-1577
> Project: Geode
>  Issue Type: Bug
>  Components: functions
>Reporter: Dan Smith
>Assignee: Dan Smith
>
> The execute methods of the function service Execution class returns a 
> ResultCollector with wildcards for the type.
> {code}  
> public ResultCollector execute(
>   Function function) throws FunctionException;
> {code}
> Wildcards are supposed to be used in APIs where the type doesn't matter, for 
> example counting the elements in a list. By returning a ResultCollector with 
> wildcards, we're essentially forcing the user to cast the result collector.
> At a minimum they should be able to pick the type of result collector
> {code}
>   public  ResultCollector execute(
>   Function function) throws FunctionException;
> {code}
> But maybe it would make more sense to parameterize Execution itself. Then the 
> compiler could ensure that the types used by withCollector and the types used 
> by execute match.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Resolved] (GEODE-2550) Improve README

2017-02-27 Thread Anthony Baker (JIRA)


 [ 
https://issues.apache.org/jira/browse/GEODE-2550?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Anthony Baker resolved GEODE-2550.
--
   Resolution: Fixed
Fix Version/s: 1.2.0

> Improve README
> --
>
> Key: GEODE-2550
> URL: https://issues.apache.org/jira/browse/GEODE-2550
> Project: Geode
>  Issue Type: Improvement
>  Components: docs
>Reporter: Anthony Baker
>Assignee: Anthony Baker
> Fix For: 1.2.0
>
>
> The project README and BUILDING could use some updates, particularly the 
> 'Geode in 5 minutes' example.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (GEODE-2550) Improve README


[ 
https://issues.apache.org/jira/browse/GEODE-2550?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15887234#comment-15887234
 ] 

ASF subversion and git services commented on GEODE-2550:


Commit b5fd6b50b18ce13cb53ef72f9fe5f363cb566ba5 in geode's branch 
refs/heads/develop from [~amb]
[ https://git-wip-us.apache.org/repos/asf?p=geode.git;h=b5fd6b5 ]

GEODE-2550 Improve README and BUILDING

This closes #407


> Improve README
> --
>
> Key: GEODE-2550
> URL: https://issues.apache.org/jira/browse/GEODE-2550
> Project: Geode
>  Issue Type: Improvement
>  Components: docs
>Reporter: Anthony Baker
>Assignee: Anthony Baker
>
> The project README and BUILDING could use some updates, particularly the 
> 'Geode in 5 minutes' example.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (GEODE-2550) Improve README


[ 
https://issues.apache.org/jira/browse/GEODE-2550?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15887236#comment-15887236
 ] 

ASF GitHub Bot commented on GEODE-2550:
---

Github user asfgit closed the pull request at:

https://github.com/apache/geode/pull/407


> Improve README
> --
>
> Key: GEODE-2550
> URL: https://issues.apache.org/jira/browse/GEODE-2550
> Project: Geode
>  Issue Type: Improvement
>  Components: docs
>Reporter: Anthony Baker
>Assignee: Anthony Baker
>
> The project README and BUILDING could use some updates, particularly the 
> 'Geode in 5 minutes' example.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] geode pull request #407: GEODE-2550 Improve README and BUILDING

2017-02-27 Thread asfgit

Github user asfgit closed the pull request at:

https://github.com/apache/geode/pull/407


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode-native pull request #36: GEODE-2494: Replace SpinLock with spinlock_mu...

Github user pivotal-jbarrett commented on a diff in the pull request:

https://github.com/apache/geode-native/pull/36#discussion_r103359636
  
--- Diff: src/cppcache/src/CqQueryVsdStats.cpp ---
@@ -20,43 +20,38 @@
 #include "CqQueryVsdStats.hpp"
 //#include "StatisticsFactory.hpp"
 
-#include 
 #include 
 
-const char* cqStatsName = (const char*)"CqQueryStatistics";
-const char* cqStatsDesc = (const char*)"Statistics for this cq query";
+#include 
+
+#include "util/concurrent/spinlock_mutex.hpp"
+
+const char* cqStatsName = "CqQueryStatistics";
+const char* cqStatsDesc = "Statistics for this cq query";
--- End diff --

Good call here though. I can actually move them into the class using 
`constexpr`, which keeps them safely scoped to this class.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (GEODE-2494) Replace SpinLock class with C++11 style BasicLockable class, spinlock_mutex.


[ 
https://issues.apache.org/jira/browse/GEODE-2494?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15887047#comment-15887047
 ] 

ASF GitHub Bot commented on GEODE-2494:
---

Github user pivotal-jbarrett commented on a diff in the pull request:

https://github.com/apache/geode-native/pull/36#discussion_r103359636
  
--- Diff: src/cppcache/src/CqQueryVsdStats.cpp ---
@@ -20,43 +20,38 @@
 #include "CqQueryVsdStats.hpp"
 //#include "StatisticsFactory.hpp"
 
-#include 
 #include 
 
-const char* cqStatsName = (const char*)"CqQueryStatistics";
-const char* cqStatsDesc = (const char*)"Statistics for this cq query";
+#include 
+
+#include "util/concurrent/spinlock_mutex.hpp"
+
+const char* cqStatsName = "CqQueryStatistics";
+const char* cqStatsDesc = "Statistics for this cq query";
--- End diff --

Good call here though. I can actually move them into the class using 
`constexpr`, which keeps them safely scoped to this class.


> Replace SpinLock class with C++11 style BasicLockable class, spinlock_mutex.
> 
>
> Key: GEODE-2494
> URL: https://issues.apache.org/jira/browse/GEODE-2494
> Project: Geode
>  Issue Type: Sub-task
>  Components: native client
>Reporter: Jacob S. Barrett
>Assignee: Jacob S. Barrett
>
> Replace {{SpinLock}} class with C++11 style 
> {{[BasicLockable|http://en.cppreference.com/w/cpp/concept/BasicLockable]}} 
> class, {{spinlock_mutex}}. You can find several public domain examples of how 
> to implement a {{spinlock_mutex}} that can be used with 
> {{[std::lock_guard|http://en.cppreference.com/w/cpp/thread/lock_guard]}}.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (GEODE-2531) Replace HostAsm::atomic* and AtomicInc with std::atomic


[ 
https://issues.apache.org/jira/browse/GEODE-2531?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15886996#comment-15886996
 ] 

ASF GitHub Bot commented on GEODE-2531:
---

Github user pivotal-jbarrett commented on a diff in the pull request:

https://github.com/apache/geode-native/pull/37#discussion_r103355539
  
--- Diff: src/cppcache/src/statistics/AtomicStatisticsImpl.cpp ---
@@ -330,7 +329,13 @@ double AtomicStatisticsImpl::_incDouble(int32_t 
offset, double delta) {
 throw IllegalArgumentException(s);
   }
 
-  return (doubleStorage[offset] += delta);
+  double expected = doubleStorage[offset];
+  double value;
+  do {
+value = expected + delta;
+  } while (!doubleStorage[offset].compare_exchange_weak(expected, value));
--- End diff --

Yes. Some of the documentation for std::atomic suck out there and don't 
clearly state that expected is updated if the compare fails.


> Replace HostAsm::atomic* and AtomicInc with std::atomic
> ---
>
> Key: GEODE-2531
> URL: https://issues.apache.org/jira/browse/GEODE-2531
> Project: Geode
>  Issue Type: Sub-task
>  Components: native client
>Reporter: Jacob S. Barrett
>Assignee: Jacob S. Barrett
>
> Replace {{HostAsm::atomic*}} and {{AtomicInc}} with 
> {{[std::atomic|http://en.cppreference.com/w/cpp/atomic/atomic]}}.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] geode-native pull request #37: GEODE-2531: Replace HostAsm::atomic with std:...

Github user pivotal-jbarrett commented on a diff in the pull request:

https://github.com/apache/geode-native/pull/37#discussion_r103355539
  
--- Diff: src/cppcache/src/statistics/AtomicStatisticsImpl.cpp ---
@@ -330,7 +329,13 @@ double AtomicStatisticsImpl::_incDouble(int32_t 
offset, double delta) {
 throw IllegalArgumentException(s);
   }
 
-  return (doubleStorage[offset] += delta);
+  double expected = doubleStorage[offset];
+  double value;
+  do {
+value = expected + delta;
+  } while (!doubleStorage[offset].compare_exchange_weak(expected, value));
--- End diff --

Yes. Some of the documentation for std::atomic suck out there and don't 
clearly state that expected is updated if the compare fails.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (GEODE-2531) Replace HostAsm::atomic* and AtomicInc with std::atomic


[ 
https://issues.apache.org/jira/browse/GEODE-2531?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15886991#comment-15886991
 ] 

ASF GitHub Bot commented on GEODE-2531:
---

Github user pivotal-jbarrett commented on a diff in the pull request:

https://github.com/apache/geode-native/pull/37#discussion_r103355323
  
--- Diff: src/cppcache/include/geode/SharedBase.hpp ---
@@ -56,11 +57,12 @@ class CPPCACHE_EXPORT SharedBase {
 
  protected:
   inline SharedBase(bool noInit) {}
+  inline SharedBase(const SharedBase&) {}
 
   virtual ~SharedBase() {}
 
  private:
-  mutable volatile int32_t m_refCount;
+  std::atomic m_refCount;
--- End diff --

It didn't appear that the `mutable` attribute was necessary for operations 
on this value. This `SharedBase` thing is a little funky so I didn't poke too 
deep. The compiler did not find any issues where a `const` function or 
reference was making changes to this value. It may be worth more investigation. 


> Replace HostAsm::atomic* and AtomicInc with std::atomic
> ---
>
> Key: GEODE-2531
> URL: https://issues.apache.org/jira/browse/GEODE-2531
> Project: Geode
>  Issue Type: Sub-task
>  Components: native client
>Reporter: Jacob S. Barrett
>Assignee: Jacob S. Barrett
>
> Replace {{HostAsm::atomic*}} and {{AtomicInc}} with 
> {{[std::atomic|http://en.cppreference.com/w/cpp/atomic/atomic]}}.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] geode-native pull request #37: GEODE-2531: Replace HostAsm::atomic with std:...

Github user pivotal-jbarrett commented on a diff in the pull request:

https://github.com/apache/geode-native/pull/37#discussion_r103355323
  
--- Diff: src/cppcache/include/geode/SharedBase.hpp ---
@@ -56,11 +57,12 @@ class CPPCACHE_EXPORT SharedBase {
 
  protected:
   inline SharedBase(bool noInit) {}
+  inline SharedBase(const SharedBase&) {}
 
   virtual ~SharedBase() {}
 
  private:
-  mutable volatile int32_t m_refCount;
+  std::atomic m_refCount;
--- End diff --

It didn't appear that the `mutable` attribute was necessary for operations 
on this value. This `SharedBase` thing is a little funky so I didn't poke too 
deep. The compiler did not find any issues where a `const` function or 
reference was making changes to this value. It may be worth more investigation. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (GEODE-2531) Replace HostAsm::atomic* and AtomicInc with std::atomic


[ 
https://issues.apache.org/jira/browse/GEODE-2531?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15886980#comment-15886980
 ] 

ASF GitHub Bot commented on GEODE-2531:
---

Github user pivotal-jbarrett commented on a diff in the pull request:

https://github.com/apache/geode-native/pull/37#discussion_r103354398
  
--- Diff: src/cppcache/include/geode/CacheStatistics.hpp ---
@@ -102,8 +102,8 @@ class CPPCACHE_EXPORT CacheStatistics : public 
SharedBase {
   virtual void setLastAccessedTime(uint32_t lat);
   virtual void setLastModifiedTime(uint32_t lmt);
 
-  volatile uint32_t m_lastAccessTime;
-  volatile uint32_t m_lastModifiedTime;
+  std::atomic m_lastAccessTime;
+  std::atomic m_lastModifiedTime;
--- End diff --

The original code was using atomic operations already, just from multiple 
platform specific sources. std::atomic gets compiled into whatever is most 
performant for the platform. I would guess that this is actually faster than 
whatever we were doing on each platform since many were function calls into 
another library. This code will get compiled to atomic put opcodes on most 
platforms (with appropriate memory synchronization between caches).


> Replace HostAsm::atomic* and AtomicInc with std::atomic
> ---
>
> Key: GEODE-2531
> URL: https://issues.apache.org/jira/browse/GEODE-2531
> Project: Geode
>  Issue Type: Sub-task
>  Components: native client
>Reporter: Jacob S. Barrett
>Assignee: Jacob S. Barrett
>
> Replace {{HostAsm::atomic*}} and {{AtomicInc}} with 
> {{[std::atomic|http://en.cppreference.com/w/cpp/atomic/atomic]}}.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] geode-native pull request #37: GEODE-2531: Replace HostAsm::atomic with std:...

Github user pivotal-jbarrett commented on a diff in the pull request:

https://github.com/apache/geode-native/pull/37#discussion_r103354398
  
--- Diff: src/cppcache/include/geode/CacheStatistics.hpp ---
@@ -102,8 +102,8 @@ class CPPCACHE_EXPORT CacheStatistics : public 
SharedBase {
   virtual void setLastAccessedTime(uint32_t lat);
   virtual void setLastModifiedTime(uint32_t lmt);
 
-  volatile uint32_t m_lastAccessTime;
-  volatile uint32_t m_lastModifiedTime;
+  std::atomic m_lastAccessTime;
+  std::atomic m_lastModifiedTime;
--- End diff --

The original code was using atomic operations already, just from multiple 
platform specific sources. std::atomic gets compiled into whatever is most 
performant for the platform. I would guess that this is actually faster than 
whatever we were doing on each platform since many were function calls into 
another library. This code will get compiled to atomic put opcodes on most 
platforms (with appropriate memory synchronization between caches).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (GEODE-2494) Replace SpinLock class with C++11 style BasicLockable class, spinlock_mutex.


[ 
https://issues.apache.org/jira/browse/GEODE-2494?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15886970#comment-15886970
 ] 

ASF GitHub Bot commented on GEODE-2494:
---

Github user pivotal-jbarrett commented on a diff in the pull request:

https://github.com/apache/geode-native/pull/36#discussion_r103353683
  
--- Diff: src/cppcache/src/LRUEntriesMap.cpp ---
@@ -20,6 +20,9 @@
 #include "MapSegment.hpp"
 #include "CacheImpl.hpp"
 
+#include 
+#include "util/concurrent/spinlock_mutex.hpp"
--- End diff --

See response to @dgkimura .


> Replace SpinLock class with C++11 style BasicLockable class, spinlock_mutex.
> 
>
> Key: GEODE-2494
> URL: https://issues.apache.org/jira/browse/GEODE-2494
> Project: Geode
>  Issue Type: Sub-task
>  Components: native client
>Reporter: Jacob S. Barrett
>Assignee: Jacob S. Barrett
>
> Replace {{SpinLock}} class with C++11 style 
> {{[BasicLockable|http://en.cppreference.com/w/cpp/concept/BasicLockable]}} 
> class, {{spinlock_mutex}}. You can find several public domain examples of how 
> to implement a {{spinlock_mutex}} that can be used with 
> {{[std::lock_guard|http://en.cppreference.com/w/cpp/thread/lock_guard]}}.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] geode-native pull request #36: GEODE-2494: Replace SpinLock with spinlock_mu...

Github user pivotal-jbarrett commented on a diff in the pull request:

https://github.com/apache/geode-native/pull/36#discussion_r103353683
  
--- Diff: src/cppcache/src/LRUEntriesMap.cpp ---
@@ -20,6 +20,9 @@
 #include "MapSegment.hpp"
 #include "CacheImpl.hpp"
 
+#include 
+#include "util/concurrent/spinlock_mutex.hpp"
--- End diff --

See response to @dgkimura .


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (GEODE-2494) Replace SpinLock class with C++11 style BasicLockable class, spinlock_mutex.


[ 
https://issues.apache.org/jira/browse/GEODE-2494?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15886968#comment-15886968
 ] 

ASF GitHub Bot commented on GEODE-2494:
---

Github user pivotal-jbarrett commented on a diff in the pull request:

https://github.com/apache/geode-native/pull/36#discussion_r103353568
  
--- Diff: src/cppcache/src/LRUList.hpp ---
@@ -20,32 +20,32 @@
  * limitations under the License.
  */
 
+#include 
+
 #include 
 #include 
-#include "SpinLock.hpp"
+
+#include "util/concurrent/spinlock_mutex.hpp"
 
 namespace apache {
 namespace geode {
 namespace client {
+
 // Bit mask for recently used
-#define RECENTLY_USED_BITS 1ul
+#define RECENTLY_USED_BITS 1u
 // Bit mask for evicted
-#define EVICTED_BITS 2ul
+#define EVICTED_BITS 2u
--- End diff --

They are macros, but they could be converted to `const` in anonymous 
namespace or even in this namespace or `constexpr` on the class itself. I will 
look at what that looks like and maybe update it.


> Replace SpinLock class with C++11 style BasicLockable class, spinlock_mutex.
> 
>
> Key: GEODE-2494
> URL: https://issues.apache.org/jira/browse/GEODE-2494
> Project: Geode
>  Issue Type: Sub-task
>  Components: native client
>Reporter: Jacob S. Barrett
>Assignee: Jacob S. Barrett
>
> Replace {{SpinLock}} class with C++11 style 
> {{[BasicLockable|http://en.cppreference.com/w/cpp/concept/BasicLockable]}} 
> class, {{spinlock_mutex}}. You can find several public domain examples of how 
> to implement a {{spinlock_mutex}} that can be used with 
> {{[std::lock_guard|http://en.cppreference.com/w/cpp/thread/lock_guard]}}.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (GEODE-2494) Replace SpinLock class with C++11 style BasicLockable class, spinlock_mutex.


[ 
https://issues.apache.org/jira/browse/GEODE-2494?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15886969#comment-15886969
 ] 

ASF GitHub Bot commented on GEODE-2494:
---

Github user pivotal-jbarrett commented on a diff in the pull request:

https://github.com/apache/geode-native/pull/36#discussion_r103353607
  
--- Diff: src/cppcache/src/MapSegment.cpp ---
@@ -316,16 +320,16 @@ GfErrType MapSegment::remove(const CacheableKeyPtr& 
key, CacheablePtr& oldValue,
  MapEntryImplPtr& me, int updateCount,
  VersionTagPtr versionTag, bool afterRemote,
  bool& isEntryFound) {
-  //  _GF_GUARD_SEGMENT;
+  //  std::lock_guard lk(m_spinlock);
--- End diff --

Yup


> Replace SpinLock class with C++11 style BasicLockable class, spinlock_mutex.
> 
>
> Key: GEODE-2494
> URL: https://issues.apache.org/jira/browse/GEODE-2494
> Project: Geode
>  Issue Type: Sub-task
>  Components: native client
>Reporter: Jacob S. Barrett
>Assignee: Jacob S. Barrett
>
> Replace {{SpinLock}} class with C++11 style 
> {{[BasicLockable|http://en.cppreference.com/w/cpp/concept/BasicLockable]}} 
> class, {{spinlock_mutex}}. You can find several public domain examples of how 
> to implement a {{spinlock_mutex}} that can be used with 
> {{[std::lock_guard|http://en.cppreference.com/w/cpp/thread/lock_guard]}}.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] geode-native pull request #36: GEODE-2494: Replace SpinLock with spinlock_mu...

Github user pivotal-jbarrett commented on a diff in the pull request:

https://github.com/apache/geode-native/pull/36#discussion_r103353607
  
--- Diff: src/cppcache/src/MapSegment.cpp ---
@@ -316,16 +320,16 @@ GfErrType MapSegment::remove(const CacheableKeyPtr& 
key, CacheablePtr& oldValue,
  MapEntryImplPtr& me, int updateCount,
  VersionTagPtr versionTag, bool afterRemote,
  bool& isEntryFound) {
-  //  _GF_GUARD_SEGMENT;
+  //  std::lock_guard lk(m_spinlock);
--- End diff --

Yup


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode-native pull request #36: GEODE-2494: Replace SpinLock with spinlock_mu...

Github user pivotal-jbarrett commented on a diff in the pull request:

https://github.com/apache/geode-native/pull/36#discussion_r103353568
  
--- Diff: src/cppcache/src/LRUList.hpp ---
@@ -20,32 +20,32 @@
  * limitations under the License.
  */
 
+#include 
+
 #include 
 #include 
-#include "SpinLock.hpp"
+
+#include "util/concurrent/spinlock_mutex.hpp"
 
 namespace apache {
 namespace geode {
 namespace client {
+
 // Bit mask for recently used
-#define RECENTLY_USED_BITS 1ul
+#define RECENTLY_USED_BITS 1u
 // Bit mask for evicted
-#define EVICTED_BITS 2ul
+#define EVICTED_BITS 2u
--- End diff --

They are macros, but they could be converted to `const` in anonymous 
namespace or even in this namespace or `constexpr` on the class itself. I will 
look at what that looks like and maybe update it.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (GEODE-2494) Replace SpinLock class with C++11 style BasicLockable class, spinlock_mutex.


[ 
https://issues.apache.org/jira/browse/GEODE-2494?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15886965#comment-15886965
 ] 

ASF GitHub Bot commented on GEODE-2494:
---

Github user pivotal-jbarrett commented on a diff in the pull request:

https://github.com/apache/geode-native/pull/36#discussion_r103353400
  
--- Diff: src/cppcache/src/MapSegment.cpp ---
@@ -19,18 +19,22 @@
 #include "TrackedMapEntry.hpp"
 #include "RegionInternal.hpp"
 #include "TableOfPrimes.hpp"
-#include "SpinLock.hpp"
 #include "Utils.hpp"
 #include "ThinClientPoolDM.hpp"
 #include "ThinClientRegion.hpp"
 #include "TombstoneExpiryHandler.hpp"
 #include 
 #include "ace/Time_Value.h"
-using namespace apache::geode::client;
 
-#define _GF_GUARD_SEGMENT SpinLockGuard mapGuard(m_spinlock)
--- End diff --

Death to macros!


> Replace SpinLock class with C++11 style BasicLockable class, spinlock_mutex.
> 
>
> Key: GEODE-2494
> URL: https://issues.apache.org/jira/browse/GEODE-2494
> Project: Geode
>  Issue Type: Sub-task
>  Components: native client
>Reporter: Jacob S. Barrett
>Assignee: Jacob S. Barrett
>
> Replace {{SpinLock}} class with C++11 style 
> {{[BasicLockable|http://en.cppreference.com/w/cpp/concept/BasicLockable]}} 
> class, {{spinlock_mutex}}. You can find several public domain examples of how 
> to implement a {{spinlock_mutex}} that can be used with 
> {{[std::lock_guard|http://en.cppreference.com/w/cpp/thread/lock_guard]}}.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] geode-native pull request #36: GEODE-2494: Replace SpinLock with spinlock_mu...

Github user pivotal-jbarrett commented on a diff in the pull request:

https://github.com/apache/geode-native/pull/36#discussion_r103353360
  
--- Diff: src/cppcache/src/LRUList.cpp ---
@@ -96,33 +102,38 @@ void LRUList::getLRUEntry(LRUListEntryPtr& result) {
 
 template 
 typename LRUList::LRUListNode*
-LRUList::getHeadNode(bool& isLast) {
-  LOCK_HEAD;
+LRUList::getHeadNode(bool* isLast) {
--- End diff --

Good questions... Adherence to Google C++ Style Guide on references vs. 
pointers. I think I did this before writing my questions about the practice on 
the dev@geode. I can back this change out.

For reference, the guide dictates that references must be const and should 
be used for in variables only. For out variable you should use pointer.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode-native pull request #36: GEODE-2494: Replace SpinLock with spinlock_mu...

Github user pivotal-jbarrett commented on a diff in the pull request:

https://github.com/apache/geode-native/pull/36#discussion_r103353400
  
--- Diff: src/cppcache/src/MapSegment.cpp ---
@@ -19,18 +19,22 @@
 #include "TrackedMapEntry.hpp"
 #include "RegionInternal.hpp"
 #include "TableOfPrimes.hpp"
-#include "SpinLock.hpp"
 #include "Utils.hpp"
 #include "ThinClientPoolDM.hpp"
 #include "ThinClientRegion.hpp"
 #include "TombstoneExpiryHandler.hpp"
 #include 
 #include "ace/Time_Value.h"
-using namespace apache::geode::client;
 
-#define _GF_GUARD_SEGMENT SpinLockGuard mapGuard(m_spinlock)
--- End diff --

Death to macros!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (GEODE-2494) Replace SpinLock class with C++11 style BasicLockable class, spinlock_mutex.


[ 
https://issues.apache.org/jira/browse/GEODE-2494?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15886962#comment-15886962
 ] 

ASF GitHub Bot commented on GEODE-2494:
---

Github user pivotal-jbarrett commented on a diff in the pull request:

https://github.com/apache/geode-native/pull/36#discussion_r103353360
  
--- Diff: src/cppcache/src/LRUList.cpp ---
@@ -96,33 +102,38 @@ void LRUList::getLRUEntry(LRUListEntryPtr& result) {
 
 template 
 typename LRUList::LRUListNode*
-LRUList::getHeadNode(bool& isLast) {
-  LOCK_HEAD;
+LRUList::getHeadNode(bool* isLast) {
--- End diff --

Good questions... Adherence to Google C++ Style Guide on references vs. 
pointers. I think I did this before writing my questions about the practice on 
the dev@geode. I can back this change out.

For reference, the guide dictates that references must be const and should 
be used for in variables only. For out variable you should use pointer.



> Replace SpinLock class with C++11 style BasicLockable class, spinlock_mutex.
> 
>
> Key: GEODE-2494
> URL: https://issues.apache.org/jira/browse/GEODE-2494
> Project: Geode
>  Issue Type: Sub-task
>  Components: native client
>Reporter: Jacob S. Barrett
>Assignee: Jacob S. Barrett
>
> Replace {{SpinLock}} class with C++11 style 
> {{[BasicLockable|http://en.cppreference.com/w/cpp/concept/BasicLockable]}} 
> class, {{spinlock_mutex}}. You can find several public domain examples of how 
> to implement a {{spinlock_mutex}} that can be used with 
> {{[std::lock_guard|http://en.cppreference.com/w/cpp/thread/lock_guard]}}.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (GEODE-2494) Replace SpinLock class with C++11 style BasicLockable class, spinlock_mutex.


[ 
https://issues.apache.org/jira/browse/GEODE-2494?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15886957#comment-15886957
 ] 

ASF GitHub Bot commented on GEODE-2494:
---

Github user pivotal-jbarrett commented on a diff in the pull request:

https://github.com/apache/geode-native/pull/36#discussion_r103352845
  
--- Diff: src/cppcache/src/LRUList.cpp ---
@@ -15,12 +15,18 @@
  * limitations under the License.
  */
 #include "LRUList.hpp"
-#include "SpinLock.hpp"
+#include "util/concurrent/spinlock_mutex.hpp"
 
-using namespace apache::geode::client;
+#include 
 
-#define LOCK_HEAD SpinLockGuard headLockGuard(m_headLock)
-#define LOCK_TAIL SpinLockGuard tailLockGuard(m_tailLock)
+namespace apache {
+namespace geode {
+namespace client {
+
+using util::concurrent::spinlock_mutex;
+
+//#define LOCK_HEAD SpinLockGuard headLockGuard(m_headLock)
+//#define LOCK_TAIL SpinLockGuard tailLockGuard(m_tailLock)
--- End diff --

Yes.


> Replace SpinLock class with C++11 style BasicLockable class, spinlock_mutex.
> 
>
> Key: GEODE-2494
> URL: https://issues.apache.org/jira/browse/GEODE-2494
> Project: Geode
>  Issue Type: Sub-task
>  Components: native client
>Reporter: Jacob S. Barrett
>Assignee: Jacob S. Barrett
>
> Replace {{SpinLock}} class with C++11 style 
> {{[BasicLockable|http://en.cppreference.com/w/cpp/concept/BasicLockable]}} 
> class, {{spinlock_mutex}}. You can find several public domain examples of how 
> to implement a {{spinlock_mutex}} that can be used with 
> {{[std::lock_guard|http://en.cppreference.com/w/cpp/thread/lock_guard]}}.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] geode-native pull request #36: GEODE-2494: Replace SpinLock with spinlock_mu...

Github user pivotal-jbarrett commented on a diff in the pull request:

https://github.com/apache/geode-native/pull/36#discussion_r103352845
  
--- Diff: src/cppcache/src/LRUList.cpp ---
@@ -15,12 +15,18 @@
  * limitations under the License.
  */
 #include "LRUList.hpp"
-#include "SpinLock.hpp"
+#include "util/concurrent/spinlock_mutex.hpp"
 
-using namespace apache::geode::client;
+#include 
 
-#define LOCK_HEAD SpinLockGuard headLockGuard(m_headLock)
-#define LOCK_TAIL SpinLockGuard tailLockGuard(m_tailLock)
+namespace apache {
+namespace geode {
+namespace client {
+
+using util::concurrent::spinlock_mutex;
+
+//#define LOCK_HEAD SpinLockGuard headLockGuard(m_headLock)
+//#define LOCK_TAIL SpinLockGuard tailLockGuard(m_tailLock)
--- End diff --

Yes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (GEODE-2494) Replace SpinLock class with C++11 style BasicLockable class, spinlock_mutex.


[ 
https://issues.apache.org/jira/browse/GEODE-2494?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15886956#comment-15886956
 ] 

ASF GitHub Bot commented on GEODE-2494:
---

Github user pivotal-jbarrett commented on a diff in the pull request:

https://github.com/apache/geode-native/pull/36#discussion_r103352781
  
--- Diff: src/cppcache/src/CqQueryVsdStats.cpp ---
@@ -108,21 +98,21 @@ CqQueryStatType::CqQueryStatType()
 

 
 CqQueryVsdStats::CqQueryVsdStats(const char* cqqueryName) {
-  CqQueryStatType* regStatType = CqQueryStatType::getInstance();
+  auto& regStatType = CqQueryStatType::getInstance();
 
-  StatisticsType* statsType = regStatType->getStatType();
+  StatisticsType* statsType = regStatType.getStatType();
 
-  GF_D_ASSERT(statsType != NULL);
+  GF_D_ASSERT(statsType != nullptr);
--- End diff --

`static_assert` is a compile time assertion. In this case the assertion 
can't be resolved at compile time since the variables and functions involved 
are not `constexpr`, templates or other compiler resolved things.


> Replace SpinLock class with C++11 style BasicLockable class, spinlock_mutex.
> 
>
> Key: GEODE-2494
> URL: https://issues.apache.org/jira/browse/GEODE-2494
> Project: Geode
>  Issue Type: Sub-task
>  Components: native client
>Reporter: Jacob S. Barrett
>Assignee: Jacob S. Barrett
>
> Replace {{SpinLock}} class with C++11 style 
> {{[BasicLockable|http://en.cppreference.com/w/cpp/concept/BasicLockable]}} 
> class, {{spinlock_mutex}}. You can find several public domain examples of how 
> to implement a {{spinlock_mutex}} that can be used with 
> {{[std::lock_guard|http://en.cppreference.com/w/cpp/thread/lock_guard]}}.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] geode-native pull request #36: GEODE-2494: Replace SpinLock with spinlock_mu...

Github user pivotal-jbarrett commented on a diff in the pull request:

https://github.com/apache/geode-native/pull/36#discussion_r103352781
  
--- Diff: src/cppcache/src/CqQueryVsdStats.cpp ---
@@ -108,21 +98,21 @@ CqQueryStatType::CqQueryStatType()
 

 
 CqQueryVsdStats::CqQueryVsdStats(const char* cqqueryName) {
-  CqQueryStatType* regStatType = CqQueryStatType::getInstance();
+  auto& regStatType = CqQueryStatType::getInstance();
 
-  StatisticsType* statsType = regStatType->getStatType();
+  StatisticsType* statsType = regStatType.getStatType();
 
-  GF_D_ASSERT(statsType != NULL);
+  GF_D_ASSERT(statsType != nullptr);
--- End diff --

`static_assert` is a compile time assertion. In this case the assertion 
can't be resolved at compile time since the variables and functions involved 
are not `constexpr`, templates or other compiler resolved things.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (GEODE-2494) Replace SpinLock class with C++11 style BasicLockable class, spinlock_mutex.


[ 
https://issues.apache.org/jira/browse/GEODE-2494?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15886954#comment-15886954
 ] 

ASF GitHub Bot commented on GEODE-2494:
---

Github user pivotal-jbarrett commented on a diff in the pull request:

https://github.com/apache/geode-native/pull/36#discussion_r103352237
  
--- Diff: src/cppcache/integration-test/testSpinLock.cpp ---
@@ -20,48 +20,44 @@
 #include "fw_dunit.hpp"
 #include 
 
+#include 
+#include 
+
 #include 
 
 #include 
 #include 
 #include 
 
-namespace apache {
-namespace geode {
-namespace client {
+namespace {
 
-CPPCACHE_EXPORT void* testSpinLockCreate();
-CPPCACHE_EXPORT void testSpinLockAcquire(void* lock);
-CPPCACHE_EXPORT void testSpinLockRelease(void* lock);
-}  // namespace client
-}  // namespace geode
-}  // namespace apache
+using apache::geode::util::concurrent::spinlock_mutex;
 
 DUNIT_TASK(s1p1, Basic)
   {
-void* lock = apache::geode::client::testSpinLockCreate();
-apache::geode::client::testSpinLockAcquire(lock);
-apache::geode::client::testSpinLockRelease(lock);
+spinlock_mutex s;
+{ std::lock_guard lk(s); }
   }
 END_TASK(Basic)
 
 perf::Semaphore* triggerA;
 perf::Semaphore* triggerB;
 perf::Semaphore* triggerM;
 
-void* lock;
+spinlock_mutex lock;
 ACE_Time_Value* btime;
 
 class ThreadA : public ACE_Task_Base {
  public:
   ThreadA() : ACE_Task_Base() {}
 
   int svc() {
-apache::geode::client::testSpinLockAcquire(lock);
-LOG("ThreadA: Acquired lock x.");
-triggerM->release();
-triggerA->acquire();
-apache::geode::client::testSpinLockRelease(lock);
+{
+  std::lock_guard lk(lock);
+  LOG("ThreadA: Acquired lock x.");
+  triggerM->release();
+  triggerA->acquire();
+}
--- End diff --

Yes.


> Replace SpinLock class with C++11 style BasicLockable class, spinlock_mutex.
> 
>
> Key: GEODE-2494
> URL: https://issues.apache.org/jira/browse/GEODE-2494
> Project: Geode
>  Issue Type: Sub-task
>  Components: native client
>Reporter: Jacob S. Barrett
>Assignee: Jacob S. Barrett
>
> Replace {{SpinLock}} class with C++11 style 
> {{[BasicLockable|http://en.cppreference.com/w/cpp/concept/BasicLockable]}} 
> class, {{spinlock_mutex}}. You can find several public domain examples of how 
> to implement a {{spinlock_mutex}} that can be used with 
> {{[std::lock_guard|http://en.cppreference.com/w/cpp/thread/lock_guard]}}.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] geode-native pull request #36: GEODE-2494: Replace SpinLock with spinlock_mu...

Github user pivotal-jbarrett commented on a diff in the pull request:

https://github.com/apache/geode-native/pull/36#discussion_r103352237
  
--- Diff: src/cppcache/integration-test/testSpinLock.cpp ---
@@ -20,48 +20,44 @@
 #include "fw_dunit.hpp"
 #include 
 
+#include 
+#include 
+
 #include 
 
 #include 
 #include 
 #include 
 
-namespace apache {
-namespace geode {
-namespace client {
+namespace {
 
-CPPCACHE_EXPORT void* testSpinLockCreate();
-CPPCACHE_EXPORT void testSpinLockAcquire(void* lock);
-CPPCACHE_EXPORT void testSpinLockRelease(void* lock);
-}  // namespace client
-}  // namespace geode
-}  // namespace apache
+using apache::geode::util::concurrent::spinlock_mutex;
 
 DUNIT_TASK(s1p1, Basic)
   {
-void* lock = apache::geode::client::testSpinLockCreate();
-apache::geode::client::testSpinLockAcquire(lock);
-apache::geode::client::testSpinLockRelease(lock);
+spinlock_mutex s;
+{ std::lock_guard lk(s); }
   }
 END_TASK(Basic)
 
 perf::Semaphore* triggerA;
 perf::Semaphore* triggerB;
 perf::Semaphore* triggerM;
 
-void* lock;
+spinlock_mutex lock;
 ACE_Time_Value* btime;
 
 class ThreadA : public ACE_Task_Base {
  public:
   ThreadA() : ACE_Task_Base() {}
 
   int svc() {
-apache::geode::client::testSpinLockAcquire(lock);
-LOG("ThreadA: Acquired lock x.");
-triggerM->release();
-triggerA->acquire();
-apache::geode::client::testSpinLockRelease(lock);
+{
+  std::lock_guard lk(lock);
+  LOG("ThreadA: Acquired lock x.");
+  triggerM->release();
+  triggerA->acquire();
+}
--- End diff --

Yes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (GEODE-2494) Replace SpinLock class with C++11 style BasicLockable class, spinlock_mutex.


[ 
https://issues.apache.org/jira/browse/GEODE-2494?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15886952#comment-15886952
 ] 

ASF GitHub Bot commented on GEODE-2494:
---

Github user pivotal-jbarrett commented on a diff in the pull request:

https://github.com/apache/geode-native/pull/36#discussion_r103352019
  
--- Diff: src/cppcache/src/MapSegment.hpp ---
@@ -164,9 +166,9 @@ class CPPCACHE_EXPORT MapSegment {
 m_entryFactory->newMapEntry(key, newEntry);
 newEntry->setValueI(newValue);
 if (m_concurrencyChecksEnabled) {
-  if (versionTag != NULLPTR && versionTag.ptr() != NULL) {
+  if (versionTag != NULLPTR && versionTag.ptr() != nullptr) {
--- End diff --

The original `NULL` is C for `0` and was allowed in C++ as an implicit cast 
to `(void *)0`.


> Replace SpinLock class with C++11 style BasicLockable class, spinlock_mutex.
> 
>
> Key: GEODE-2494
> URL: https://issues.apache.org/jira/browse/GEODE-2494
> Project: Geode
>  Issue Type: Sub-task
>  Components: native client
>Reporter: Jacob S. Barrett
>Assignee: Jacob S. Barrett
>
> Replace {{SpinLock}} class with C++11 style 
> {{[BasicLockable|http://en.cppreference.com/w/cpp/concept/BasicLockable]}} 
> class, {{spinlock_mutex}}. You can find several public domain examples of how 
> to implement a {{spinlock_mutex}} that can be used with 
> {{[std::lock_guard|http://en.cppreference.com/w/cpp/thread/lock_guard]}}.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] geode-native pull request #36: GEODE-2494: Replace SpinLock with spinlock_mu...

Github user pivotal-jbarrett commented on a diff in the pull request:

https://github.com/apache/geode-native/pull/36#discussion_r103352019
  
--- Diff: src/cppcache/src/MapSegment.hpp ---
@@ -164,9 +166,9 @@ class CPPCACHE_EXPORT MapSegment {
 m_entryFactory->newMapEntry(key, newEntry);
 newEntry->setValueI(newValue);
 if (m_concurrencyChecksEnabled) {
-  if (versionTag != NULLPTR && versionTag.ptr() != NULL) {
+  if (versionTag != NULLPTR && versionTag.ptr() != nullptr) {
--- End diff --

The original `NULL` is C for `0` and was allowed in C++ as an implicit cast 
to `(void *)0`.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (GEODE-2494) Replace SpinLock class with C++11 style BasicLockable class, spinlock_mutex.


[ 
https://issues.apache.org/jira/browse/GEODE-2494?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15886950#comment-15886950
 ] 

ASF GitHub Bot commented on GEODE-2494:
---

Github user pivotal-jbarrett commented on a diff in the pull request:

https://github.com/apache/geode-native/pull/36#discussion_r103351894
  
--- Diff: src/cppcache/src/MapSegment.hpp ---
@@ -164,9 +166,9 @@ class CPPCACHE_EXPORT MapSegment {
 m_entryFactory->newMapEntry(key, newEntry);
 newEntry->setValueI(newValue);
 if (m_concurrencyChecksEnabled) {
-  if (versionTag != NULLPTR && versionTag.ptr() != NULL) {
+  if (versionTag != NULLPTR && versionTag.ptr() != nullptr) {
--- End diff --

`NULLPTR` is Geode constant from "empty" `SharedPtr`. 

`nullptr` is C++11 for "empty" pointer.


> Replace SpinLock class with C++11 style BasicLockable class, spinlock_mutex.
> 
>
> Key: GEODE-2494
> URL: https://issues.apache.org/jira/browse/GEODE-2494
> Project: Geode
>  Issue Type: Sub-task
>  Components: native client
>Reporter: Jacob S. Barrett
>Assignee: Jacob S. Barrett
>
> Replace {{SpinLock}} class with C++11 style 
> {{[BasicLockable|http://en.cppreference.com/w/cpp/concept/BasicLockable]}} 
> class, {{spinlock_mutex}}. You can find several public domain examples of how 
> to implement a {{spinlock_mutex}} that can be used with 
> {{[std::lock_guard|http://en.cppreference.com/w/cpp/thread/lock_guard]}}.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] geode-native pull request #36: GEODE-2494: Replace SpinLock with spinlock_mu...

Github user pivotal-jbarrett commented on a diff in the pull request:

https://github.com/apache/geode-native/pull/36#discussion_r103351894
  
--- Diff: src/cppcache/src/MapSegment.hpp ---
@@ -164,9 +166,9 @@ class CPPCACHE_EXPORT MapSegment {
 m_entryFactory->newMapEntry(key, newEntry);
 newEntry->setValueI(newValue);
 if (m_concurrencyChecksEnabled) {
-  if (versionTag != NULLPTR && versionTag.ptr() != NULL) {
+  if (versionTag != NULLPTR && versionTag.ptr() != nullptr) {
--- End diff --

`NULLPTR` is Geode constant from "empty" `SharedPtr`. 

`nullptr` is C++11 for "empty" pointer.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (GEODE-2494) Replace SpinLock class with C++11 style BasicLockable class, spinlock_mutex.


[ 
https://issues.apache.org/jira/browse/GEODE-2494?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15886948#comment-15886948
 ] 

ASF GitHub Bot commented on GEODE-2494:
---

Github user pivotal-jbarrett commented on a diff in the pull request:

https://github.com/apache/geode-native/pull/36#discussion_r103351757
  
--- Diff: src/cppcache/src/MapSegment.hpp ---
@@ -41,6 +40,9 @@
 #include 
 #include "TombstoneList.hpp"
 #include 
+
+#include "util/concurrent/spinlock_mutex.hpp"
--- End diff --

`"` includes are for "local" includes. If `spinlock_mutex` was an external 
library then `<` would be used.

If "down here" means why the sub directories, then the answer is to be 
consistent with the namespace. The utility of that is debatable.



> Replace SpinLock class with C++11 style BasicLockable class, spinlock_mutex.
> 
>
> Key: GEODE-2494
> URL: https://issues.apache.org/jira/browse/GEODE-2494
> Project: Geode
>  Issue Type: Sub-task
>  Components: native client
>Reporter: Jacob S. Barrett
>Assignee: Jacob S. Barrett
>
> Replace {{SpinLock}} class with C++11 style 
> {{[BasicLockable|http://en.cppreference.com/w/cpp/concept/BasicLockable]}} 
> class, {{spinlock_mutex}}. You can find several public domain examples of how 
> to implement a {{spinlock_mutex}} that can be used with 
> {{[std::lock_guard|http://en.cppreference.com/w/cpp/thread/lock_guard]}}.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] geode-native pull request #36: GEODE-2494: Replace SpinLock with spinlock_mu...

Github user pivotal-jbarrett commented on a diff in the pull request:

https://github.com/apache/geode-native/pull/36#discussion_r103351757
  
--- Diff: src/cppcache/src/MapSegment.hpp ---
@@ -41,6 +40,9 @@
 #include 
 #include "TombstoneList.hpp"
 #include 
+
+#include "util/concurrent/spinlock_mutex.hpp"
--- End diff --

`"` includes are for "local" includes. If `spinlock_mutex` was an external 
library then `<` would be used.

If "down here" means why the sub directories, then the answer is to be 
consistent with the namespace. The utility of that is debatable.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (GEODE-2494) Replace SpinLock class with C++11 style BasicLockable class, spinlock_mutex.


[ 
https://issues.apache.org/jira/browse/GEODE-2494?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15886945#comment-15886945
 ] 

ASF GitHub Bot commented on GEODE-2494:
---

Github user pivotal-jbarrett commented on a diff in the pull request:

https://github.com/apache/geode-native/pull/36#discussion_r103351422
  
--- Diff: src/tests/cpp/security/Security.cpp ---
@@ -1014,12 +1016,15 @@ int32_t Security::doEntryOperations() {
 reinterpret_cast(valBuf),
 static_cast(strlen(valBuf)));
 int32_t *val =
-(int32_t *)(dynCast(tmpValue)->value());
+const_cast(reinterpret_cast(
+dynCast(tmpValue)->value()));
--- End diff --

`dynCast` is something Geode does to unwrap and rewrap the nasty 
`SharedPtr` so I am not going to touch that. The `const_cast(reinterpret_cast` is the real change here to do the C++ 
version of `(int32_t*)` in the original. `dynCast` results in a `const X *`, so 
the obvious `reinterpret_cast` is actually a compiler error because 
`reinterpret_cast` can not drop modifiers like `const`. The `const_cast` then 
drops the modifier.


> Replace SpinLock class with C++11 style BasicLockable class, spinlock_mutex.
> 
>
> Key: GEODE-2494
> URL: https://issues.apache.org/jira/browse/GEODE-2494
> Project: Geode
>  Issue Type: Sub-task
>  Components: native client
>Reporter: Jacob S. Barrett
>Assignee: Jacob S. Barrett
>
> Replace {{SpinLock}} class with C++11 style 
> {{[BasicLockable|http://en.cppreference.com/w/cpp/concept/BasicLockable]}} 
> class, {{spinlock_mutex}}. You can find several public domain examples of how 
> to implement a {{spinlock_mutex}} that can be used with 
> {{[std::lock_guard|http://en.cppreference.com/w/cpp/thread/lock_guard]}}.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] geode-native pull request #36: GEODE-2494: Replace SpinLock with spinlock_mu...

Github user pivotal-jbarrett commented on a diff in the pull request:

https://github.com/apache/geode-native/pull/36#discussion_r103351422
  
--- Diff: src/tests/cpp/security/Security.cpp ---
@@ -1014,12 +1016,15 @@ int32_t Security::doEntryOperations() {
 reinterpret_cast(valBuf),
 static_cast(strlen(valBuf)));
 int32_t *val =
-(int32_t *)(dynCast(tmpValue)->value());
+const_cast(reinterpret_cast(
+dynCast(tmpValue)->value()));
--- End diff --

`dynCast` is something Geode does to unwrap and rewrap the nasty 
`SharedPtr` so I am not going to touch that. The `const_cast(reinterpret_cast` is the real change here to do the C++ 
version of `(int32_t*)` in the original. `dynCast` results in a `const X *`, so 
the obvious `reinterpret_cast` is actually a compiler error because 
`reinterpret_cast` can not drop modifiers like `const`. The `const_cast` then 
drops the modifier.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (GEODE-2494) Replace SpinLock class with C++11 style BasicLockable class, spinlock_mutex.


[ 
https://issues.apache.org/jira/browse/GEODE-2494?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15886929#comment-15886929
 ] 

ASF GitHub Bot commented on GEODE-2494:
---

Github user pivotal-jbarrett commented on a diff in the pull request:

https://github.com/apache/geode-native/pull/36#discussion_r103350612
  
--- Diff: src/cppcache/src/LRUEntriesMap.cpp ---
@@ -297,7 +295,7 @@ GfErrType LRUEntriesMap::put(const CacheableKeyPtr& key,
   }
 }
 // SpinLock& lock = segmentRPtr->getSpinLock();
-// SpinLockGuard mapGuard( lock );
+// std::lock_guard mapGuard( lock );
--- End diff --

Yes, thanks!


> Replace SpinLock class with C++11 style BasicLockable class, spinlock_mutex.
> 
>
> Key: GEODE-2494
> URL: https://issues.apache.org/jira/browse/GEODE-2494
> Project: Geode
>  Issue Type: Sub-task
>  Components: native client
>Reporter: Jacob S. Barrett
>Assignee: Jacob S. Barrett
>
> Replace {{SpinLock}} class with C++11 style 
> {{[BasicLockable|http://en.cppreference.com/w/cpp/concept/BasicLockable]}} 
> class, {{spinlock_mutex}}. You can find several public domain examples of how 
> to implement a {{spinlock_mutex}} that can be used with 
> {{[std::lock_guard|http://en.cppreference.com/w/cpp/thread/lock_guard]}}.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (GEODE-2494) Replace SpinLock class with C++11 style BasicLockable class, spinlock_mutex.


[ 
https://issues.apache.org/jira/browse/GEODE-2494?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15886928#comment-15886928
 ] 

ASF GitHub Bot commented on GEODE-2494:
---

Github user pivotal-jbarrett commented on a diff in the pull request:

https://github.com/apache/geode-native/pull/36#discussion_r103350610
  
--- Diff: src/tests/cpp/security/Security.cpp ---
@@ -41,16 +41,18 @@
 
 #include "security/CredentialGenerator.hpp"
 
-namespace FwkSecurity {
+#include 
+#include 
+
+namespace apache {
+namespace geode {
+namespace client {
+namespace testframework {
+namespace security {
--- End diff --

Probably but I didn't want to change their current scope without 
understanding the test. I wanted to just correct the warning around `using 
namespace`.


> Replace SpinLock class with C++11 style BasicLockable class, spinlock_mutex.
> 
>
> Key: GEODE-2494
> URL: https://issues.apache.org/jira/browse/GEODE-2494
> Project: Geode
>  Issue Type: Sub-task
>  Components: native client
>Reporter: Jacob S. Barrett
>Assignee: Jacob S. Barrett
>
> Replace {{SpinLock}} class with C++11 style 
> {{[BasicLockable|http://en.cppreference.com/w/cpp/concept/BasicLockable]}} 
> class, {{spinlock_mutex}}. You can find several public domain examples of how 
> to implement a {{spinlock_mutex}} that can be used with 
> {{[std::lock_guard|http://en.cppreference.com/w/cpp/thread/lock_guard]}}.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] geode-native pull request #36: GEODE-2494: Replace SpinLock with spinlock_mu...

Github user pivotal-jbarrett commented on a diff in the pull request:

https://github.com/apache/geode-native/pull/36#discussion_r103350612
  
--- Diff: src/cppcache/src/LRUEntriesMap.cpp ---
@@ -297,7 +295,7 @@ GfErrType LRUEntriesMap::put(const CacheableKeyPtr& key,
   }
 }
 // SpinLock& lock = segmentRPtr->getSpinLock();
-// SpinLockGuard mapGuard( lock );
+// std::lock_guard mapGuard( lock );
--- End diff --

Yes, thanks!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode-native pull request #36: GEODE-2494: Replace SpinLock with spinlock_mu...

Github user pivotal-jbarrett commented on a diff in the pull request:

https://github.com/apache/geode-native/pull/36#discussion_r103350610
  
--- Diff: src/tests/cpp/security/Security.cpp ---
@@ -41,16 +41,18 @@
 
 #include "security/CredentialGenerator.hpp"
 
-namespace FwkSecurity {
+#include 
+#include 
+
+namespace apache {
+namespace geode {
+namespace client {
+namespace testframework {
+namespace security {
--- End diff --

Probably but I didn't want to change their current scope without 
understanding the test. I wanted to just correct the warning around `using 
namespace`.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Re: Review Request 57064: GEODE-2460: update dependency versions

2017-02-27 Thread Anthony Baker


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57064/#review166976
---


Ship it!




Ship It!

- Anthony Baker


On Feb. 28, 2017, 12:03 a.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57064/
> ---
> 
> (Updated Feb. 28, 2017, 12:03 a.m.)
> 
> 
> Review request for geode, Anthony Baker, Jared Stewart, Mark Bretl, Udo 
> Kohlmeyer, and Dan Smith.
> 
> 
> Bugs: GEODE-2460
> https://issues.apache.org/jira/browse/GEODE-2460
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2460: update dependency versions
> 
> * com.fasterxml.jackson.core:jackson-annotation:2.8.6
> * com.fasterxml.jackson.core:jackson-core:2.8.6
> * com.fasterxml.jackson.core:jackson-databind:2.8.6
> * com.fasterxml.jackson.module:jackson-module-scala_2.10:2.8.6
> * org.eclipse.persistence:javax.persistence:2.1.1
> * fix dependency versions in NOTICE files
> 
> 
> Diffs
> -
> 
>   geode-assembly/src/main/dist/NOTICE 
> 2cdda713bcb7d1ba5c09407f658273515dd63b8c 
>   geode-pulse/src/main/webapp/META-INF/NOTICE 
> 166a9ee94d0a0cdd41a4266fe3c042d4a1155624 
>   geode-web-api/src/main/webapp/META-INF/NOTICE 
> 8e1775bbcde41b909c817b68ac2eca05d4626c44 
>   gradle/dependency-versions.properties 
> be828af14826ff63adee18717de063d87e2f1c6c 
> 
> Diff: https://reviews.apache.org/r/57064/diff/
> 
> 
> Testing
> ---
> 
> precheckin passed (100% green)
> 2nd precheckin in progress
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>

Re: Review Request 57064: GEODE-2460: update dependency versions


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57064/
---

(Updated Feb. 28, 2017, 12:03 a.m.)


Review request for geode, Anthony Baker, Jared Stewart, Mark Bretl, Udo 
Kohlmeyer, and Dan Smith.


Changes
---

Fix version numbers in NOTICE files. Remove update of json4s.


Bugs: GEODE-2460
https://issues.apache.org/jira/browse/GEODE-2460


Repository: geode


Description (updated)
---

GEODE-2460: update dependency versions

* com.fasterxml.jackson.core:jackson-annotation:2.8.6
* com.fasterxml.jackson.core:jackson-core:2.8.6
* com.fasterxml.jackson.core:jackson-databind:2.8.6
* com.fasterxml.jackson.module:jackson-module-scala_2.10:2.8.6
* org.eclipse.persistence:javax.persistence:2.1.1
* fix dependency versions in NOTICE files


Diffs (updated)
-

  geode-assembly/src/main/dist/NOTICE 2cdda713bcb7d1ba5c09407f658273515dd63b8c 
  geode-pulse/src/main/webapp/META-INF/NOTICE 
166a9ee94d0a0cdd41a4266fe3c042d4a1155624 
  geode-web-api/src/main/webapp/META-INF/NOTICE 
8e1775bbcde41b909c817b68ac2eca05d4626c44 
  gradle/dependency-versions.properties 
be828af14826ff63adee18717de063d87e2f1c6c 

Diff: https://reviews.apache.org/r/57064/diff/


Testing (updated)
---

precheckin passed (100% green)
2nd precheckin in progress


Thanks,

Kirk Lund

Re: Review Request 57064: GEODE-2460: update dependency versions



> On Feb. 27, 2017, 11:13 p.m., Kirk Lund wrote:
> > gradle/dependency-versions.properties, line 62
> > 
> >
> > I suspect this can be removed as an explicit dependency

Dropped this from update. I'll remove it in GEODE-2461.


- Kirk


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57064/#review166961
---


On Feb. 27, 2017, 11:12 p.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57064/
> ---
> 
> (Updated Feb. 27, 2017, 11:12 p.m.)
> 
> 
> Review request for geode, Anthony Baker, Jared Stewart, Mark Bretl, Udo 
> Kohlmeyer, and Dan Smith.
> 
> 
> Bugs: GEODE-2460
> https://issues.apache.org/jira/browse/GEODE-2460
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2460: update dependency versions
> 
> * com.fasterxml.jackson.core:jackson-annotation:2.8.6
> * com.fasterxml.jackson.core:jackson-core:2.8.6
> * com.fasterxml.jackson.core:jackson-databind:2.8.6
> * com.fasterxml.jackson.module:jackson-module-scala_2.10:2.8.6
> * org.eclipse.persistence:javax.persistence:2.1.1
> * org.json4s:json4s-ast_2.10:3.5.0
> 
> 
> Diffs
> -
> 
>   gradle/dependency-versions.properties 
> be828af14826ff63adee18717de063d87e2f1c6c 
> 
> Diff: https://reviews.apache.org/r/57064/diff/
> 
> 
> Testing
> ---
> 
> precheckin passed (100% green)
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>

Re: Review Request 57064: GEODE-2460: update dependency versions

2017-02-27 Thread ASF subversion and git services (JIRA)



> On Feb. 27, 2017, 11:31 p.m., Anthony Baker wrote:
> > Can you fix these references too?  Thanks!
> > 
> > ```
> > ~/code/incubator-geode (develop)$ git grep -i "Jackson Core 2.8.2"
> > geode-assembly/src/main/dist/NOTICE:Jackson Core 2.8.2
> > geode-web-api/src/main/webapp/META-INF/NOTICE:Jackson Core 2.8.2
> > ```
> 
> Kirk Lund wrote:
> Yep, thanks for finding those!

I found a couple more NOTICE files that had incorrect versions so I'm combining 
the fix for those with this changeset.


- Kirk


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57064/#review166967
---


On Feb. 27, 2017, 11:12 p.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57064/
> ---
> 
> (Updated Feb. 27, 2017, 11:12 p.m.)
> 
> 
> Review request for geode, Anthony Baker, Jared Stewart, Mark Bretl, Udo 
> Kohlmeyer, and Dan Smith.
> 
> 
> Bugs: GEODE-2460
> https://issues.apache.org/jira/browse/GEODE-2460
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2460: update dependency versions
> 
> * com.fasterxml.jackson.core:jackson-annotation:2.8.6
> * com.fasterxml.jackson.core:jackson-core:2.8.6
> * com.fasterxml.jackson.core:jackson-databind:2.8.6
> * com.fasterxml.jackson.module:jackson-module-scala_2.10:2.8.6
> * org.eclipse.persistence:javax.persistence:2.1.1
> * org.json4s:json4s-ast_2.10:3.5.0
> 
> 
> Diffs
> -
> 
>   gradle/dependency-versions.properties 
> be828af14826ff63adee18717de063d87e2f1c6c 
> 
> Diff: https://reviews.apache.org/r/57064/diff/
> 
> 
> Testing
> ---
> 
> precheckin passed (100% green)
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>

[jira] [Commented] (GEODE-2538) Lucene query with values should not invoke cache loader


[ 
https://issues.apache.org/jira/browse/GEODE-2538?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15886772#comment-15886772
 ] 

ASF subversion and git services commented on GEODE-2538:


Commit 712d87f791906cbd1c11dd0f4655032dabf57755 in geode's branch 
refs/heads/develop from [~upthewaterspout]
[ https://git-wip-us.apache.org/repos/asf?p=geode.git;h=712d87f ]

GEODE-2538: Don't invoke a cache loader when fetching values for a lucene query

Instead of using getAll, fetch the values of a lucene query using a
function that calls getEntry. We can then avoid invoking the cache
loader.


> Lucene query with values should not invoke cache loader
> ---
>
> Key: GEODE-2538
> URL: https://issues.apache.org/jira/browse/GEODE-2538
> Project: Geode
>  Issue Type: Improvement
>  Components: lucene
>Reporter: Dan Smith
>Assignee: Dan Smith
>
> Lucene queries use getAll to fetch pages when returning values.
> If an entry is destroyed after the query has found the list of keys, but 
> before the query finds the values, the getAll with trigger a cache loader and 
> reload the value. This may be unexpected behavior.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

Re: Review Request 57064: GEODE-2460: update dependency versions



> On Feb. 27, 2017, 11:31 p.m., Anthony Baker wrote:
> > Can you fix these references too?  Thanks!
> > 
> > ```
> > ~/code/incubator-geode (develop)$ git grep -i "Jackson Core 2.8.2"
> > geode-assembly/src/main/dist/NOTICE:Jackson Core 2.8.2
> > geode-web-api/src/main/webapp/META-INF/NOTICE:Jackson Core 2.8.2
> > ```

Yep, thanks for finding those!


- Kirk


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57064/#review166967
---


On Feb. 27, 2017, 11:12 p.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57064/
> ---
> 
> (Updated Feb. 27, 2017, 11:12 p.m.)
> 
> 
> Review request for geode, Anthony Baker, Jared Stewart, Mark Bretl, Udo 
> Kohlmeyer, and Dan Smith.
> 
> 
> Bugs: GEODE-2460
> https://issues.apache.org/jira/browse/GEODE-2460
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2460: update dependency versions
> 
> * com.fasterxml.jackson.core:jackson-annotation:2.8.6
> * com.fasterxml.jackson.core:jackson-core:2.8.6
> * com.fasterxml.jackson.core:jackson-databind:2.8.6
> * com.fasterxml.jackson.module:jackson-module-scala_2.10:2.8.6
> * org.eclipse.persistence:javax.persistence:2.1.1
> * org.json4s:json4s-ast_2.10:3.5.0
> 
> 
> Diffs
> -
> 
>   gradle/dependency-versions.properties 
> be828af14826ff63adee18717de063d87e2f1c6c 
> 
> Diff: https://reviews.apache.org/r/57064/diff/
> 
> 
> Testing
> ---
> 
> precheckin passed (100% green)
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>

Re: Review Request 57057: GEODE-2538: Don't invoke a cache loader when fetching values for a lucene query

2017-02-27 Thread Dan Smith



> On Feb. 27, 2017, 10:29 p.m., Barry Oglesby wrote:
> > geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/results/LuceneGetPageFunction.java,
> >  line 75
> > 
> >
> > I'm not sure if you want to deal with this, but the getValue call will 
> > deserialize the value on the server, then it will be reserializeed to send 
> > to the client.

Good point. I think I should fix that, but I'll do it as a separate commit so 
that I can get this fix in before doing that optimization.


- Dan


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57057/#review166946
---


On Feb. 25, 2017, 1:05 a.m., Dan Smith wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57057/
> ---
> 
> (Updated Feb. 25, 2017, 1:05 a.m.)
> 
> 
> Review request for geode, Barry Oglesby, Jason Huynh, nabarun nag, and 
> xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Instead of using getAll, fetch the values of a lucene query using a
> function that calls getEntry. We can then avoid invoking the cache
> loader.
> 
> 
> Diffs
> -
> 
>   
> geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneQueryImpl.java
>  b50996be8eeb2677537ba7756e33fc199601b3fc 
>   
> geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneServiceImpl.java
>  cf7b2c9a3e0a78417fa192ceee57441e005536ea 
>   
> geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/PageableLuceneQueryResultsImpl.java
>  5c5d8250cf9c31f0d29964fe5f12f3bae779500d 
>   
> geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/results/LuceneGetPageFunction.java
>  PRE-CREATION 
>   
> geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/results/MapResultCollector.java
>  PRE-CREATION 
>   
> geode-lucene/src/test/java/org/apache/geode/cache/lucene/LuceneQueriesIntegrationTest.java
>  01bdca40a59422dc0700ff22e867db7717a374ea 
>   
> geode-lucene/src/test/java/org/apache/geode/cache/lucene/PaginationDUnitTest.java
>  cfde4f23305e3e4c6419268d32b04e2c9db4f9db 
>   
> geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/LuceneQueryImplJUnitTest.java
>  9f826d586479e9e56b9e7130a9f23e7f706693a7 
>   
> geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/PageableLuceneQueryResultsImplJUnitTest.java
>  35a4c9169714ff766a655255d471cb639b7a0404 
>   
> geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/results/LuceneGetPageFunctionTest.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/57057/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dan Smith
> 
>

Re: Review Request 57064: GEODE-2460: update dependency versions

2017-02-27 Thread Anthony Baker


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57064/#review166967
---



Can you fix these references too?  Thanks!

```
~/code/incubator-geode (develop)$ git grep -i "Jackson Core 2.8.2"
geode-assembly/src/main/dist/NOTICE:Jackson Core 2.8.2
geode-web-api/src/main/webapp/META-INF/NOTICE:Jackson Core 2.8.2
```

- Anthony Baker


On Feb. 27, 2017, 11:12 p.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57064/
> ---
> 
> (Updated Feb. 27, 2017, 11:12 p.m.)
> 
> 
> Review request for geode, Anthony Baker, Jared Stewart, Mark Bretl, Udo 
> Kohlmeyer, and Dan Smith.
> 
> 
> Bugs: GEODE-2460
> https://issues.apache.org/jira/browse/GEODE-2460
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2460: update dependency versions
> 
> * com.fasterxml.jackson.core:jackson-annotation:2.8.6
> * com.fasterxml.jackson.core:jackson-core:2.8.6
> * com.fasterxml.jackson.core:jackson-databind:2.8.6
> * com.fasterxml.jackson.module:jackson-module-scala_2.10:2.8.6
> * org.eclipse.persistence:javax.persistence:2.1.1
> * org.json4s:json4s-ast_2.10:3.5.0
> 
> 
> Diffs
> -
> 
>   gradle/dependency-versions.properties 
> be828af14826ff63adee18717de063d87e2f1c6c 
> 
> Diff: https://reviews.apache.org/r/57064/diff/
> 
> 
> Testing
> ---
> 
> precheckin passed (100% green)
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>

[jira] [Updated] (GEODE-2555) Region Management docs page refers to the wrong field


 [ 
https://issues.apache.org/jira/browse/GEODE-2555?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Galen O'Sullivan updated GEODE-2555:

Priority: Minor  (was: Major)

> Region Management docs page refers to the wrong field
> -
>
> Key: GEODE-2555
> URL: https://issues.apache.org/jira/browse/GEODE-2555
> Project: Geode
>  Issue Type: Bug
>  Components: docs
>Affects Versions: 1.1.0
>Reporter: Galen O'Sullivan
>Priority: Minor
>
> In 
> https://geode.apache.org/docs/guide/11/basic_config/data_regions/managing_data_regions.html
> {code}
>   
> {code}
> should be 
> {code}
>   
> {code}
> The region tag doesn't support an id attribute, only refid. Copypasting the 
> given config gives me an error.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Updated] (GEODE-2555) Region Management docs page refers to the wrong field


 [ 
https://issues.apache.org/jira/browse/GEODE-2555?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Galen O'Sullivan updated GEODE-2555:

Summary: Region Management docs page refers to the wrong field  (was: 
Region Management refers to the wrong field)

> Region Management docs page refers to the wrong field
> -
>
> Key: GEODE-2555
> URL: https://issues.apache.org/jira/browse/GEODE-2555
> Project: Geode
>  Issue Type: Bug
>  Components: docs
>Affects Versions: 1.1.0
>Reporter: Galen O'Sullivan
>
> In 
> https://geode.apache.org/docs/guide/11/basic_config/data_regions/managing_data_regions.html
> {code}
>   
> {code}
> should be 
> {code}
>   
> {code}
> The region tag doesn't support an id attribute, only refid. Copypasting the 
> given config gives me an error.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Updated] (GEODE-2555) Region Management docs page refers to the wrong field


 [ 
https://issues.apache.org/jira/browse/GEODE-2555?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Galen O'Sullivan updated GEODE-2555:

Affects Version/s: 1.1.0

> Region Management docs page refers to the wrong field
> -
>
> Key: GEODE-2555
> URL: https://issues.apache.org/jira/browse/GEODE-2555
> Project: Geode
>  Issue Type: Bug
>  Components: docs
>Affects Versions: 1.1.0
>Reporter: Galen O'Sullivan
>
> In 
> https://geode.apache.org/docs/guide/11/basic_config/data_regions/managing_data_regions.html
> {code}
>   
> {code}
> should be 
> {code}
>   
> {code}
> The region tag doesn't support an id attribute, only refid. Copypasting the 
> given config gives me an error.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Updated] (GEODE-2555) Region Management refers to the wrong field


 [ 
https://issues.apache.org/jira/browse/GEODE-2555?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Galen O'Sullivan updated GEODE-2555:

Summary: Region Management refers to the wrong field  (was: Region 
Management refers to a field that doesn't work.)

> Region Management refers to the wrong field
> ---
>
> Key: GEODE-2555
> URL: https://issues.apache.org/jira/browse/GEODE-2555
> Project: Geode
>  Issue Type: Bug
>  Components: docs
>Reporter: Galen O'Sullivan
>
> In 
> https://geode.apache.org/docs/guide/11/basic_config/data_regions/managing_data_regions.html
> {code}
>   
> {code}
> should be 
> {code}
>   
> {code}
> The region tag doesn't support an id attribute, only refid. Copypasting the 
> given config gives me an error.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Created] (GEODE-2555) Region Management refers to a field that doesn't work.

Galen O'Sullivan created GEODE-2555:
---

 Summary: Region Management refers to a field that doesn't work.
 Key: GEODE-2555
 URL: https://issues.apache.org/jira/browse/GEODE-2555
 Project: Geode
  Issue Type: Bug
  Components: docs
Reporter: Galen O'Sullivan


In 
https://geode.apache.org/docs/guide/11/basic_config/data_regions/managing_data_regions.html
{code}
  
{code}
should be 
{code}
  
{code}

The region tag doesn't support an id attribute, only refid. Copypasting the 
given config gives me an error.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Updated] (GEODE-2553) Lucene search hangs on recreated region with no data

2017-02-27 Thread Diane Hardman (JIRA)


 [ 
https://issues.apache.org/jira/browse/GEODE-2553?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Diane Hardman updated GEODE-2553:
-
Attachment: server50505.log

This is the server logfile generated by GemFire.

> Lucene search hangs on recreated region with no data
> 
>
> Key: GEODE-2553
> URL: https://issues.apache.org/jira/browse/GEODE-2553
> Project: Geode
>  Issue Type: Bug
>  Components: lucene
>Reporter: Diane Hardman
> Fix For: 1.2.0
>
> Attachments: server50505.log, stack.log
>
>
> While manually testing in gfsh the process of deleting Lucene indexes, 
> deleting the region, creating new indexes and a new empty region, I was able 
> to hang gfsh while doing a Lucene search on the new region with no data.
> Here are the steps I used:
> _ __
>/ _/ __/ __/ // /
>   / /  __/ /___  /_  / _  / 
>  / /__/ / /  _/ / // /  
> /__/_/  /__/_//_/1.2.0-SNAPSHOT
> Monitor and Manage Apache Geode
> gfsh>start locator --name=locator1 --port=12345
> gfsh>start server --name=server50505 --server-port=50505 
> --locators=localhost[12345] --start-rest-api --http-service-port=8080 
> --http-service-bind-address=localhost
> gfsh>create lucene index --name=testIndex --region=testRegion 
> --field=__REGION_VALUE_FIELD
> gfsh>create lucene index --name=testIndex2 --region=testRegion 
> gfsh>list lucene indexes --with-stats
> gfsh>create region --name=testRegion --type=PARTITION_PERSISTENT
> gfsh>put --key=1 --value=value1 --region=testRegion
> gfsh>put --key=2 --value=value2 --region=testRegion
> gfsh>put --key=3 --value=value3 --region=testRegion
> gfsh>search lucene --name=testIndex --region=/testRegion 
> --queryStrings=value* --defaultField=__REGION_VALUE_FIELD
> gfsh>search lucene --name=testIndex2 --region=/testRegion 
> --queryStrings=value* --defaultField=__REGION_VALUE_FIELD
> gfsh>destroy lucene index --region=/testRegion --name=testIndex
> gfsh>list lucene indexes --with-stats
> gfsh>search lucene --name=testIndex2 --region=/testRegion 
> --queryStrings=value* --defaultField=__REGION_VALUE_FIELD
> gfsh>search lucene --name=testIndex --region=/testRegion 
> --queryStrings=value* --defaultField=__REGION_VALUE_FIELD
> 
> gfsh>destroy lucene index --region=/testRegion
> gfsh>list lucene indexes --with-stats
> gfsh>destroy region --name=/testRegion
> gfsh>create lucene index --name=testIndex --region=testRegion 
> gfsh>create lucene index --name=testIndex2 --region=testRegion 
> --field=__REGION_VALUE_FIELD
> gfsh>list lucene indexes --with-stats
> gfsh>create region --name=testRegion --type=PARTITION_PERSISTENT
> gfsh>search lucene --name=testIndex --region=/testRegion 
> --queryStrings=value* --defaultField=__REGION_VALUE_FIELD
> The gfsh process hangs at this point.
> I'll attach the stacktrace for the server.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Updated] (GEODE-2553) Lucene search hangs on recreated region with no data

2017-02-27 Thread Diane Hardman (JIRA)


 [ 
https://issues.apache.org/jira/browse/GEODE-2553?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Diane Hardman updated GEODE-2553:
-
Attachment: stack.log

This is the stack trace acquired by running jstack on the PID for the server. 

> Lucene search hangs on recreated region with no data
> 
>
> Key: GEODE-2553
> URL: https://issues.apache.org/jira/browse/GEODE-2553
> Project: Geode
>  Issue Type: Bug
>  Components: lucene
>Reporter: Diane Hardman
> Fix For: 1.2.0
>
> Attachments: stack.log
>
>
> While manually testing in gfsh the process of deleting Lucene indexes, 
> deleting the region, creating new indexes and a new empty region, I was able 
> to hang gfsh while doing a Lucene search on the new region with no data.
> Here are the steps I used:
> _ __
>/ _/ __/ __/ // /
>   / /  __/ /___  /_  / _  / 
>  / /__/ / /  _/ / // /  
> /__/_/  /__/_//_/1.2.0-SNAPSHOT
> Monitor and Manage Apache Geode
> gfsh>start locator --name=locator1 --port=12345
> gfsh>start server --name=server50505 --server-port=50505 
> --locators=localhost[12345] --start-rest-api --http-service-port=8080 
> --http-service-bind-address=localhost
> gfsh>create lucene index --name=testIndex --region=testRegion 
> --field=__REGION_VALUE_FIELD
> gfsh>create lucene index --name=testIndex2 --region=testRegion 
> gfsh>list lucene indexes --with-stats
> gfsh>create region --name=testRegion --type=PARTITION_PERSISTENT
> gfsh>put --key=1 --value=value1 --region=testRegion
> gfsh>put --key=2 --value=value2 --region=testRegion
> gfsh>put --key=3 --value=value3 --region=testRegion
> gfsh>search lucene --name=testIndex --region=/testRegion 
> --queryStrings=value* --defaultField=__REGION_VALUE_FIELD
> gfsh>search lucene --name=testIndex2 --region=/testRegion 
> --queryStrings=value* --defaultField=__REGION_VALUE_FIELD
> gfsh>destroy lucene index --region=/testRegion --name=testIndex
> gfsh>list lucene indexes --with-stats
> gfsh>search lucene --name=testIndex2 --region=/testRegion 
> --queryStrings=value* --defaultField=__REGION_VALUE_FIELD
> gfsh>search lucene --name=testIndex --region=/testRegion 
> --queryStrings=value* --defaultField=__REGION_VALUE_FIELD
> 
> gfsh>destroy lucene index --region=/testRegion
> gfsh>list lucene indexes --with-stats
> gfsh>destroy region --name=/testRegion
> gfsh>create lucene index --name=testIndex --region=testRegion 
> gfsh>create lucene index --name=testIndex2 --region=testRegion 
> --field=__REGION_VALUE_FIELD
> gfsh>list lucene indexes --with-stats
> gfsh>create region --name=testRegion --type=PARTITION_PERSISTENT
> gfsh>search lucene --name=testIndex --region=/testRegion 
> --queryStrings=value* --defaultField=__REGION_VALUE_FIELD
> The gfsh process hangs at this point.
> I'll attach the stacktrace for the server.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Created] (GEODE-2554) Geode incubator docs are still up

Galen O'Sullivan created GEODE-2554:
---

 Summary: Geode incubator docs are still up
 Key: GEODE-2554
 URL: https://issues.apache.org/jira/browse/GEODE-2554
 Project: Geode
  Issue Type: Bug
  Components: docs
Reporter: Galen O'Sullivan


Search engines still direct users to the Geode incubating docs, which are at:

https://geode.apache.org/docs/guide/basic_config/data_regions/managing_data_regions.html

The most recent docs have an 11 in the URL:

https://geode.apache.org/docs/guide/11/basic_config/data_regions/managing_data_regions.html

The old docs should either be taken down, or the path made to refer to whatever 
the latest docs are. That way visitors won't get stuck on an ever increasingly 
stale docs site.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Created] (GEODE-2553) Lucene search hangs on recreated region with no data

2017-02-27 Thread Diane Hardman (JIRA)

Diane Hardman created GEODE-2553:


 Summary: Lucene search hangs on recreated region with no data
 Key: GEODE-2553
 URL: https://issues.apache.org/jira/browse/GEODE-2553
 Project: Geode
  Issue Type: Bug
  Components: lucene
Reporter: Diane Hardman
 Fix For: 1.2.0


While manually testing in gfsh the process of deleting Lucene indexes, deleting 
the region, creating new indexes and a new empty region, I was able to hang 
gfsh while doing a Lucene search on the new region with no data.
Here are the steps I used:
_ __
   / _/ __/ __/ // /
  / /  __/ /___  /_  / _  / 
 / /__/ / /  _/ / // /  
/__/_/  /__/_//_/1.2.0-SNAPSHOT

Monitor and Manage Apache Geode
gfsh>start locator --name=locator1 --port=12345

gfsh>start server --name=server50505 --server-port=50505 
--locators=localhost[12345] --start-rest-api --http-service-port=8080 
--http-service-bind-address=localhost

gfsh>create lucene index --name=testIndex --region=testRegion 
--field=__REGION_VALUE_FIELD

gfsh>create lucene index --name=testIndex2 --region=testRegion 
gfsh>list lucene indexes --with-stats

gfsh>create region --name=testRegion --type=PARTITION_PERSISTENT

gfsh>put --key=1 --value=value1 --region=testRegion

gfsh>put --key=2 --value=value2 --region=testRegion

gfsh>put --key=3 --value=value3 --region=testRegion

gfsh>search lucene --name=testIndex --region=/testRegion --queryStrings=value* 
--defaultField=__REGION_VALUE_FIELD

gfsh>search lucene --name=testIndex2 --region=/testRegion --queryStrings=value* 
--defaultField=__REGION_VALUE_FIELD

gfsh>destroy lucene index --region=/testRegion --name=testIndex

gfsh>list lucene indexes --with-stats

gfsh>search lucene --name=testIndex2 --region=/testRegion --queryStrings=value* 
--defaultField=__REGION_VALUE_FIELD

gfsh>search lucene --name=testIndex --region=/testRegion --queryStrings=value* 
--defaultField=__REGION_VALUE_FIELD


gfsh>destroy lucene index --region=/testRegion

gfsh>list lucene indexes --with-stats

gfsh>destroy region --name=/testRegion

gfsh>create lucene index --name=testIndex --region=testRegion 

gfsh>create lucene index --name=testIndex2 --region=testRegion 
--field=__REGION_VALUE_FIELD
gfsh>list lucene indexes --with-stats

gfsh>create region --name=testRegion --type=PARTITION_PERSISTENT
gfsh>search lucene --name=testIndex --region=/testRegion --queryStrings=value* 
--defaultField=__REGION_VALUE_FIELD

The gfsh process hangs at this point.

I'll attach the stacktrace for the server.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

Re: Review Request 57064: GEODE-2460: update dependency versions


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57064/#review166961
---




gradle/dependency-versions.properties (line 62)


I suspect this can be removed as an explicit dependency


- Kirk Lund


On Feb. 27, 2017, 11:12 p.m., Kirk Lund wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57064/
> ---
> 
> (Updated Feb. 27, 2017, 11:12 p.m.)
> 
> 
> Review request for geode, Anthony Baker, Jared Stewart, Mark Bretl, Udo 
> Kohlmeyer, and Dan Smith.
> 
> 
> Bugs: GEODE-2460
> https://issues.apache.org/jira/browse/GEODE-2460
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> GEODE-2460: update dependency versions
> 
> * com.fasterxml.jackson.core:jackson-annotation:2.8.6
> * com.fasterxml.jackson.core:jackson-core:2.8.6
> * com.fasterxml.jackson.core:jackson-databind:2.8.6
> * com.fasterxml.jackson.module:jackson-module-scala_2.10:2.8.6
> * org.eclipse.persistence:javax.persistence:2.1.1
> * org.json4s:json4s-ast_2.10:3.5.0
> 
> 
> Diffs
> -
> 
>   gradle/dependency-versions.properties 
> be828af14826ff63adee18717de063d87e2f1c6c 
> 
> Diff: https://reviews.apache.org/r/57064/diff/
> 
> 
> Testing
> ---
> 
> precheckin passed (100% green)
> 
> 
> Thanks,
> 
> Kirk Lund
> 
>

Review Request 57064: GEODE-2460: update dependency versions


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57064/
---

Review request for geode, Anthony Baker, Jared Stewart, Mark Bretl, Udo 
Kohlmeyer, and Dan Smith.


Bugs: GEODE-2460
https://issues.apache.org/jira/browse/GEODE-2460


Repository: geode


Description
---

GEODE-2460: update dependency versions

* com.fasterxml.jackson.core:jackson-annotation:2.8.6
* com.fasterxml.jackson.core:jackson-core:2.8.6
* com.fasterxml.jackson.core:jackson-databind:2.8.6
* com.fasterxml.jackson.module:jackson-module-scala_2.10:2.8.6
* org.eclipse.persistence:javax.persistence:2.1.1
* org.json4s:json4s-ast_2.10:3.5.0


Diffs
-

  gradle/dependency-versions.properties 
be828af14826ff63adee18717de063d87e2f1c6c 

Diff: https://reviews.apache.org/r/57064/diff/


Testing
---

precheckin passed (100% green)


Thanks,

Kirk Lund

[Spring CI] Spring Data GemFire > Nightly-ApacheGeode > #485 was SUCCESSFUL (with 1679 tests). Change made by John Blum.

2017-02-27 Thread Spring CI


---
Spring Data GemFire > Nightly-ApacheGeode > #485 was successful (rerun once).
---
This build was rerun by John Blum.
1681 tests in total.

https://build.spring.io/browse/SGF-NAG-485/




--
Code Changes
--
John Blum (84e671946b3b14e9b762862ba4a73dead175810d):

>SGF-597 - Set Apache Geode dependency to version 1.1.0 (GA release).

John Blum (7a893e1f1b6e3f967f09e2c8cb713a9156688f46):

>SGF-597 - Reset Artifact Repository for apache-geode build snapshots to 
>repo.spring.io/libs-snapshot.



--
This message is automatically generated by Atlassian Bamboo

[Spring CI] Spring Data GemFire > Nightly-ApacheGeode > #486 was SUCCESSFUL (with 1679 tests)

2017-02-27 Thread Spring CI


---
Spring Data GemFire > Nightly-ApacheGeode > #486 was successful.
---
Scheduled
1681 tests in total.

https://build.spring.io/browse/SGF-NAG-486/





--
Tests
--
Fixed Tests (1)
   - ApacheGeodeSecurityManagerGeodeSecurityIntegrationTests: Authorized user

--
This message is automatically generated by Atlassian Bamboo

Re: Review Request 57057: GEODE-2538: Don't invoke a cache loader when fetching values for a lucene query

2017-02-27 Thread Barry Oglesby


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57057/#review166946
---




geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/results/LuceneGetPageFunction.java
 (line 75)


I'm not sure if you want to deal with this, but the getValue call will 
deserialize the value on the server, then it will be reserializeed to send to 
the client.


- Barry Oglesby


On Feb. 25, 2017, 1:05 a.m., Dan Smith wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57057/
> ---
> 
> (Updated Feb. 25, 2017, 1:05 a.m.)
> 
> 
> Review request for geode, Barry Oglesby, Jason Huynh, nabarun nag, and 
> xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Instead of using getAll, fetch the values of a lucene query using a
> function that calls getEntry. We can then avoid invoking the cache
> loader.
> 
> 
> Diffs
> -
> 
>   
> geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneQueryImpl.java
>  b50996be8eeb2677537ba7756e33fc199601b3fc 
>   
> geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneServiceImpl.java
>  cf7b2c9a3e0a78417fa192ceee57441e005536ea 
>   
> geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/PageableLuceneQueryResultsImpl.java
>  5c5d8250cf9c31f0d29964fe5f12f3bae779500d 
>   
> geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/results/LuceneGetPageFunction.java
>  PRE-CREATION 
>   
> geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/results/MapResultCollector.java
>  PRE-CREATION 
>   
> geode-lucene/src/test/java/org/apache/geode/cache/lucene/LuceneQueriesIntegrationTest.java
>  01bdca40a59422dc0700ff22e867db7717a374ea 
>   
> geode-lucene/src/test/java/org/apache/geode/cache/lucene/PaginationDUnitTest.java
>  cfde4f23305e3e4c6419268d32b04e2c9db4f9db 
>   
> geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/LuceneQueryImplJUnitTest.java
>  9f826d586479e9e56b9e7130a9f23e7f706693a7 
>   
> geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/PageableLuceneQueryResultsImplJUnitTest.java
>  35a4c9169714ff766a655255d471cb639b7a0404 
>   
> geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/results/LuceneGetPageFunctionTest.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/57057/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dan Smith
> 
>

[jira] [Reopened] (GEODE-2428) Add support for LinkedHashMap in DataSerializer

2017-02-27 Thread Darrel Schneider (JIRA)


 [ 
https://issues.apache.org/jira/browse/GEODE-2428?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Darrel Schneider reopened GEODE-2428:
-

I think the javadocs on these new methods need to make two more things clear:
  1. readLinkedHashMap will always create a LinkedHashMap that uses insertion 
order. Even if the one serialized by writeLinkedHashMap was access order.
  2. writeLinkedHashMap is not used by DataSerializer.writeObject to serialize 
a LinkedHashMap. The other static methods on DataSerializer (for example 
writeHashMap) are used by DataSerializer.writeObject so I think this "exception 
to the rule" should be called out in javadocs.

It would be nice if writeObject did call writeLinkedHashMap but that would 
introduce backward compatibility issues. The reason this was not done at the 
time LinkedHashSet support was added was because of the "access" vs. 
"insertion" order issue #1.


> Add support for LinkedHashMap in DataSerializer
> ---
>
> Key: GEODE-2428
> URL: https://issues.apache.org/jira/browse/GEODE-2428
> Project: Geode
>  Issue Type: Improvement
>Reporter: Avinash Dongre
>Assignee: Avinash Dongre
> Fix For: 1.2.0
>
>
> DataSerializer should also support serialization and de-serialization of 
> DataSerializer



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Resolved] (GEODE-2404) Add API to destroy a region containing lucene indexes

2017-02-27 Thread Barry Oglesby (JIRA)


 [ 
https://issues.apache.org/jira/browse/GEODE-2404?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Barry Oglesby resolved GEODE-2404.
--
   Resolution: Fixed
Fix Version/s: 1.2.0

> Add API to destroy a region containing lucene indexes
> -
>
> Key: GEODE-2404
> URL: https://issues.apache.org/jira/browse/GEODE-2404
> Project: Geode
>  Issue Type: New Feature
>  Components: lucene
>Reporter: Barry Oglesby
> Fix For: 1.2.0
>
> Attachments: DestroyRegionMultipleMembersFunction.java
>
>
> h2. Description
> An application {{Region}} containing {{LuceneIndexes}} should be able to be 
> destroyed.
> There are several options, including:
> - Invoke one API to destroy both the application {{Region}} and its 
> {{LuceneIndexes}}
> - Invoke two API:
> ## destroy the {{LuceneIndexes}}
> ## destroy application {{Region}} as it is done currently
> h3. One API
> In this case, we would need a callback on {{LuceneService}} to destroy the 
> {{LuceneIndexes}} before destroying the application {{Region}} like:
> {noformat}
> public void beforeDestroyRegion(Region region);
> {noformat}
> This API would get all the {{LuceneIndexes}} for the application {{Region}}, 
> then destroy each one. See the *Two API* section below for details on 
> destroying a {{LuceneIndex}}.
> Without changes to the way {{PartitionedRegions}} are destroyed, this causes 
> an issue though.
> The current behavior of {{PartitionedRegion destroyRegion}} is to first check 
> for colocated children. If there are any, the call fails.
> There are two options for adding the call to destroy the {{LuceneIndexes}}:
> # check for colocated children
> # invoke {{LuceneService beforeDestroyRegion}} to remove the {{LuceneIndexes}}
> # do the rest of the destroy
> 
> # invoke {{LuceneService beforeDestroyRegion}} to remove the {{LuceneIndexes}}
> # check for colocated children
> # do the rest of the destroy
> Both of these options are problematic in different ways.
> In the case of a {{PartitionedRegion}} with {{LuceneIndexes}}, there are 
> colocated children, so the first option would cause the {{destroyRegion}} 
> call to fail; the second option would succeed. I don't think the first option 
> should fail since the colocated children are internal {{Regions}} that the 
> application knows nothing about.
> In the case of a {{PartitionedRegion}} defining {{LuceneIndexes}} and having 
> an {{AsyncEventQueue}}, there are colocated children, so the first option 
> would cause the {{destroyRegion}} call to fail. This is ok since one of the 
> children is an application-known {{AsyncEventQueue}}. The second option would 
> fail in a bad way. It would first remove the {{LuceneIndexes}}, then fail the 
> colocated children check, so the {{destroyRegion}} call would fail. In this 
> case, the application {{Region}} doesn't get destroyed but its 
> {{LuceneIndexes}} do. This would be bad.
> One option would be to look into changing the check for colocated children to 
> check for application-defined (or not hidden) colocated children. Then the 
> code would be something like:
> # check for application-defined colocated children
> # invoke LuceneService beforeDestroyRegion to remove the LuceneIndexes
> # do the rest of the destroy
> I think this would be ok in both cases.
> h3. Two API
> The destroy API on {{LuceneIndex}} would be something like:
> {noformat}
> public void destroy();
> {noformat}
> Destroying each {{LuceneIndex}} would require:
> # destroying the chunk {{Region}}
> # destroying the file {{Region}}
> # destroying the {{AsyncEventQueue}} which would require:
> ## retrieving and stopping the {{AsyncEventQueue's}} underlying 
> {{GatewaySender}} (there probably should be stop API on {{AsyncEventQueue}} 
> which does this)
> ## removing the id from the application {{Region's AsyncEventQueue}} ids
> ## destroying the {{AsyncEventQueue}} (this destroys the underlying 
> {{GatewaySender}} and removes it from the {{GemFireCacheImpl's}} collection 
> of {{GatewaySenders}})
> ## removing the {{AsyncEventQueue}} from the {{GemFireCacheImpl's}} 
> collection of {{AsyncEventQueues}} (this should be included in the destroy 
> method above)
> # removing {{LuceneIndex}} from {{LuceneService's}} map of indexes
> I also think the API on {{LuceneService}} should be something like:
> {noformat}
> public void destroyIndexes(String regionPath);
> public void destroyIndex(String indexName, String regionPath);
> {noformat}
> These methods would get the appropriate {{LuceneIndex(es)}} and invoke 
> destroy on them. Then they would remove the index(es) from the 
> {{LuceneService's}} collection of {{LuceneIndexes}}.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

Re: Review Request 57057: GEODE-2538: Don't invoke a cache loader when fetching values for a lucene query

2017-02-27 Thread nabarun nag


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57057/#review166949
---


Ship it!




Ship It!

- nabarun nag


On Feb. 25, 2017, 1:05 a.m., Dan Smith wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57057/
> ---
> 
> (Updated Feb. 25, 2017, 1:05 a.m.)
> 
> 
> Review request for geode, Barry Oglesby, Jason Huynh, nabarun nag, and 
> xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Instead of using getAll, fetch the values of a lucene query using a
> function that calls getEntry. We can then avoid invoking the cache
> loader.
> 
> 
> Diffs
> -
> 
>   
> geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneQueryImpl.java
>  b50996be8eeb2677537ba7756e33fc199601b3fc 
>   
> geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneServiceImpl.java
>  cf7b2c9a3e0a78417fa192ceee57441e005536ea 
>   
> geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/PageableLuceneQueryResultsImpl.java
>  5c5d8250cf9c31f0d29964fe5f12f3bae779500d 
>   
> geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/results/LuceneGetPageFunction.java
>  PRE-CREATION 
>   
> geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/results/MapResultCollector.java
>  PRE-CREATION 
>   
> geode-lucene/src/test/java/org/apache/geode/cache/lucene/LuceneQueriesIntegrationTest.java
>  01bdca40a59422dc0700ff22e867db7717a374ea 
>   
> geode-lucene/src/test/java/org/apache/geode/cache/lucene/PaginationDUnitTest.java
>  cfde4f23305e3e4c6419268d32b04e2c9db4f9db 
>   
> geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/LuceneQueryImplJUnitTest.java
>  9f826d586479e9e56b9e7130a9f23e7f706693a7 
>   
> geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/PageableLuceneQueryResultsImplJUnitTest.java
>  35a4c9169714ff766a655255d471cb639b7a0404 
>   
> geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/results/LuceneGetPageFunctionTest.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/57057/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dan Smith
> 
>

Re: Review Request 57057: GEODE-2538: Don't invoke a cache loader when fetching values for a lucene query

2017-02-27 Thread nabarun nag


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57057/#review166948
---




geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/results/LuceneGetPageFunctionTest.java
 (line 39)


Should the class name end with JUnitTest


- nabarun nag


On Feb. 25, 2017, 1:05 a.m., Dan Smith wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57057/
> ---
> 
> (Updated Feb. 25, 2017, 1:05 a.m.)
> 
> 
> Review request for geode, Barry Oglesby, Jason Huynh, nabarun nag, and 
> xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Instead of using getAll, fetch the values of a lucene query using a
> function that calls getEntry. We can then avoid invoking the cache
> loader.
> 
> 
> Diffs
> -
> 
>   
> geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneQueryImpl.java
>  b50996be8eeb2677537ba7756e33fc199601b3fc 
>   
> geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneServiceImpl.java
>  cf7b2c9a3e0a78417fa192ceee57441e005536ea 
>   
> geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/PageableLuceneQueryResultsImpl.java
>  5c5d8250cf9c31f0d29964fe5f12f3bae779500d 
>   
> geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/results/LuceneGetPageFunction.java
>  PRE-CREATION 
>   
> geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/results/MapResultCollector.java
>  PRE-CREATION 
>   
> geode-lucene/src/test/java/org/apache/geode/cache/lucene/LuceneQueriesIntegrationTest.java
>  01bdca40a59422dc0700ff22e867db7717a374ea 
>   
> geode-lucene/src/test/java/org/apache/geode/cache/lucene/PaginationDUnitTest.java
>  cfde4f23305e3e4c6419268d32b04e2c9db4f9db 
>   
> geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/LuceneQueryImplJUnitTest.java
>  9f826d586479e9e56b9e7130a9f23e7f706693a7 
>   
> geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/PageableLuceneQueryResultsImplJUnitTest.java
>  35a4c9169714ff766a655255d471cb639b7a0404 
>   
> geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/results/LuceneGetPageFunctionTest.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/57057/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dan Smith
> 
>

Re: [DISCUSS] changes to Redis implementation

2017-02-27 Thread Dan Smith

>
> Given that the feature is still labeled experimental, do our backwards
> compatibility constraints apply?
>

Actually, it doesn't look like it is marked with @Experimental or is
described as experimental in the docs.

That said, I think maybe it *should* have been marked as experimental
because I don't think it's completely baked or in common use. I'm not sure
how we want to deal with that. I'm generally in favor of switching this
model for storing hashes and sets, because I think that's the more common
use case. Maybe we just need to document that 1.2 breaks the upgrade from
1.1 for redis data?

-Dan

Re: Review Request 57057: GEODE-2538: Don't invoke a cache loader when fetching values for a lucene query

2017-02-27 Thread Jason Huynh


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57057/#review166943
---


Fix it, then Ship it!




Ship It!


geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/PageableLuceneQueryResultsImpl.java
 (line 92)


Would using the keys.size() instead of the scores.size() be better here?



geode-lucene/src/test/java/org/apache/geode/cache/lucene/LuceneQueriesIntegrationTest.java
 (line 389)


Maybe extract a variable for page size?


- Jason Huynh


On Feb. 25, 2017, 1:05 a.m., Dan Smith wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57057/
> ---
> 
> (Updated Feb. 25, 2017, 1:05 a.m.)
> 
> 
> Review request for geode, Barry Oglesby, Jason Huynh, nabarun nag, and 
> xiaojian zhou.
> 
> 
> Repository: geode
> 
> 
> Description
> ---
> 
> Instead of using getAll, fetch the values of a lucene query using a
> function that calls getEntry. We can then avoid invoking the cache
> loader.
> 
> 
> Diffs
> -
> 
>   
> geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneQueryImpl.java
>  b50996be8eeb2677537ba7756e33fc199601b3fc 
>   
> geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneServiceImpl.java
>  cf7b2c9a3e0a78417fa192ceee57441e005536ea 
>   
> geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/PageableLuceneQueryResultsImpl.java
>  5c5d8250cf9c31f0d29964fe5f12f3bae779500d 
>   
> geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/results/LuceneGetPageFunction.java
>  PRE-CREATION 
>   
> geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/results/MapResultCollector.java
>  PRE-CREATION 
>   
> geode-lucene/src/test/java/org/apache/geode/cache/lucene/LuceneQueriesIntegrationTest.java
>  01bdca40a59422dc0700ff22e867db7717a374ea 
>   
> geode-lucene/src/test/java/org/apache/geode/cache/lucene/PaginationDUnitTest.java
>  cfde4f23305e3e4c6419268d32b04e2c9db4f9db 
>   
> geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/LuceneQueryImplJUnitTest.java
>  9f826d586479e9e56b9e7130a9f23e7f706693a7 
>   
> geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/PageableLuceneQueryResultsImplJUnitTest.java
>  35a4c9169714ff766a655255d471cb639b7a0404 
>   
> geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/results/LuceneGetPageFunctionTest.java
>  PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/57057/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Dan Smith
> 
>

Re: [DISCUSS] changes to Redis implementation

2017-02-27 Thread Dan Smith

>
> Stick the operation+params into the EntryEventImpl that follows the
> operation through the system and in the BucketRegion subclass pull out that
> data and send it instead of the event's newValue.


Yeah, but that only works if your events are always played in the same
order. In the case of GII, WAN, replicated regions with concurrency checks,
and client subscriptions that is not necessarily true. For example with GII
if a new update comes in, it may show up on the recipient before the
original value. In that case we just keep the new value. The existing delta
framework handles these cases, so I agree with Bruce that it's probably
better to build off of that framework.

I do think there is a lot of room to build a better API on top of the
existing delta implementation and we can probably make an API that looks
like "simple operation replication."

-Dan

On Mon, Feb 27, 2017 at 11:39 AM, Bruce Schuchardt 
wrote:

> I think in this case it would be replacing the messages used to replicate
> changes with ones that send the operation + parameters instead of the
> modified entry's new value.  That could be done by creating a new subclass
> of BucketRegion.  Stick the operation+params into the EntryEventImpl that
> follows the operation through the system and in the BucketRegion subclass
> pull out that data and send it instead of the event's newValue.
>
>
> Le 2/27/2017 à 11:13 AM, Real Wes a écrit :
>
>> I'm not following what a "simple operation replication framework" is and
>> how it applies to the Redis API. If you replicate operations, you still
>> need to update the data at some point, causing a synchronous replication
>> event so as to provide HA. What is, in more detail, a "simple operation
>> replication framework"?
>>
>> Regards,
>> Wes Williams
>> Sent from mobile phone
>>
>> On Feb 27, 2017, at 2:07 PM, Bruce Schuchardt 
>>> wrote:
>>>
>>> What I like about using the existing delta-prop mechanism is it will
>>> also extend to client subscriptions & WAN.  It would take a lot of work to
>>> propagate Redis commands through those paths.
>>>
>>> Le 2/27/2017 à 10:35 AM, Hitesh Khamesra a écrit :
 The simplicity of Redis API making this problem (delta update)most
 apparent. But,  I would imagine many Geode apps has a similar use case.

 -Hitesh

From: Michael Stolz 
   To: dev@geode.apache.org
   Sent: Monday, February 27, 2017 9:06 AM
   Subject: Re: [DISCUSS] changes to Redis implementation
 It does seem like the operations will often be much smaller than
 the data
 they are operating on.
 It is almost the classic "move the code to the data" pattern.

 --
 Mike Stolz
 Principal Engineer, GemFire Product Manager
 Mobile: +1-631-835-4771

 On Mon, Feb 27, 2017 at 10:51 AM, Udo Kohlmeyer 
 wrote:

 Delta could provide us a mechanism to replicate only what is required.
>
> I wonder if we could not create a simple operation replication
> framework.
> Rather than writing a potential large amounts of code for delta, we
> replicate only the operation.
>
>
> On 2/27/17 07:18, Wes Williams wrote:
>>
>> Replicating a whole collection because of 1 change does not really
>> make
>>
>>> too much sense.<<
>>>
>> I agree but won't delta replication prevent sending the entire
>> collection
>> across the wire?
>>
>> *Wes Williams | Pivotal Advisory **Data Engineer*
>> 781.606.0325
>> http://pivotal.io/big-data/pivotal-gemfire
>>
>> On Mon, Feb 27, 2017 at 10:08 AM, Udo Kohlmeyer <
>> ukohlme...@pivotal.io>
>> wrote:
>>
>> I've quickly gone through the changes for the pull request.
>>
>>> The most significant change of this pull request is that the
>>> collections
>>> that initially were regions are single collections (not distributed).
>>> That
>>> said, this is something that we've been discussing. The one thing
>>> that I
>>> wonder about is, what will the performance look like when the
>>> collections
>>> become really large? Replicating a whole collection because of 1
>>> change
>>> does not really make too much sense.
>>>
>>> Maybe this implementation becomes the catalyst for future
>>> improvements.
>>>
>>> --Udo
>>>
>>>
>>>
>>> On 2/24/17 15:25, Bruce Schuchardt wrote:
>>>
>>> Gregory Green has posted a pull request that warrants discussion. It
>>>
 improves performance for Sets and Hashes by altering the storage
 format
 for
 these collections.  As such it will not permit a rolling upgrade,
 though
 the Redis adapter is labelled "experimental" so maybe that's okay.

 https://github.com/apache/geode/pull/404

 The PR also fixes GEODE-2469, inability to handle hash keys having
 colons.

 There was some di

Re: [DISCUSS] changes to Redis implementation

2017-02-27 Thread Anthony Baker

Given that the feature is still labeled experimental, do our backwards 
compatibility constraints apply?

Anthony


> On Feb 27, 2017, at 12:10 PM, Swapnil Bawaskar  wrote:
> 
> Accepting this pull request as-is will break backwards compatibility. I
> think if the new behavior is desired, it should be based on some
> configuration.
> 
> On Mon, Feb 27, 2017 at 11:40 AM Bruce Schuchardt  >
> wrote:
> 
>> I think in this case it would be replacing the messages used to
>> replicate changes with ones that send the operation + parameters instead
>> of the modified entry's new value.  That could be done by creating a new
>> subclass of BucketRegion.  Stick the operation+params into the
>> EntryEventImpl that follows the operation through the system and in the
>> BucketRegion subclass pull out that data and send it instead of the
>> event's newValue.
>> 
>> Le 2/27/2017 à 11:13 AM, Real Wes a écrit :
>>> I'm not following what a "simple operation replication framework" is and
>> how it applies to the Redis API. If you replicate operations, you still
>> need to update the data at some point, causing a synchronous replication
>> event so as to provide HA. What is, in more detail, a "simple operation
>> replication framework"?
>>> 
>>> Regards,
>>> Wes Williams
>>> Sent from mobile phone
>>> 
 On Feb 27, 2017, at 2:07 PM, Bruce Schuchardt 
>> wrote:
 
 What I like about using the existing delta-prop mechanism is it will
>> also extend to client subscriptions & WAN.  It would take a lot of work to
>> propagate Redis commands through those paths.
 
> Le 2/27/2017 à 10:35 AM, Hitesh Khamesra a écrit :
> The simplicity of Redis API making this problem (delta update)most
>> apparent. But,  I would imagine many Geode apps has a similar use case.
> 
> -Hitesh
> 
>   From: Michael Stolz 
>  To: dev@geode.apache.org
>  Sent: Monday, February 27, 2017 9:06 AM
>  Subject: Re: [DISCUSS] changes to Redis implementation
>It does seem like the operations will often be much smaller than
>> the data
> they are operating on.
> It is almost the classic "move the code to the data" pattern.
> 
> --
> Mike Stolz
> Principal Engineer, GemFire Product Manager
> Mobile: +1-631-835-4771 <(631)%20835-4771>
> 
> On Mon, Feb 27, 2017 at 10:51 AM, Udo Kohlmeyer  
>>> 
> wrote:
> 
>> Delta could provide us a mechanism to replicate only what is required.
>> 
>> I wonder if we could not create a simple operation replication
>> framework.
>> Rather than writing a potential large amounts of code for delta, we
>> replicate only the operation.
>> 
>> 
>>> On 2/27/17 07:18, Wes Williams wrote:
>>> 
>>> Replicating a whole collection because of 1 change does not really
>> make
 too much sense.<<
>>> I agree but won't delta replication prevent sending the entire
>> collection
>>> across the wire?
>>> 
>>> *Wes Williams | Pivotal Advisory **Data Engineer*
>>> 781.606.0325 <(781)%20606-0325>
>>> http://pivotal.io/big-data/pivotal-gemfire 
>>> 
>>> 
>>> On Mon, Feb 27, 2017 at 10:08 AM, Udo Kohlmeyer <
>> ukohlme...@pivotal.io >
>>> wrote:
>>> 
>>> I've quickly gone through the changes for the pull request.
 The most significant change of this pull request is that the
>> collections
 that initially were regions are single collections (not
>> distributed).
 That
 said, this is something that we've been discussing. The one thing
>> that I
 wonder about is, what will the performance look like when the
>> collections
 become really large? Replicating a whole collection because of 1
>> change
 does not really make too much sense.
 
 Maybe this implementation becomes the catalyst for future
>> improvements.
 
 --Udo
 
 
 
 On 2/24/17 15:25, Bruce Schuchardt wrote:
 
 Gregory Green has posted a pull request that warrants discussion. It
> improves performance for Sets and Hashes by altering the storage
>> format
> for
> these collections.  As such it will not permit a rolling upgrade,
>> though
> the Redis adapter is labelled "experimental" so maybe that's okay.
> 
> https://github.com/apache/geode/pull/404 
> 
> 
> The PR also fixes GEODE-2469, inability to handle hash keys having
> colons.
> 
> There was some discussion about altering the storage format that
>> was
> initiated by Hitesh.  Personally I think Gregory's changes are
>> better
> than
> the current implementation and we should accept them, though I
>> haven't
> gone

Re: [DISCUSS] changes to Redis implementation

2017-02-27 Thread Swapnil Bawaskar

Accepting this pull request as-is will break backwards compatibility. I
think if the new behavior is desired, it should be based on some
configuration.

On Mon, Feb 27, 2017 at 11:40 AM Bruce Schuchardt 
wrote:

> I think in this case it would be replacing the messages used to
> replicate changes with ones that send the operation + parameters instead
> of the modified entry's new value.  That could be done by creating a new
> subclass of BucketRegion.  Stick the operation+params into the
> EntryEventImpl that follows the operation through the system and in the
> BucketRegion subclass pull out that data and send it instead of the
> event's newValue.
>
> Le 2/27/2017 à 11:13 AM, Real Wes a écrit :
> > I'm not following what a "simple operation replication framework" is and
> how it applies to the Redis API. If you replicate operations, you still
> need to update the data at some point, causing a synchronous replication
> event so as to provide HA. What is, in more detail, a "simple operation
> replication framework"?
> >
> > Regards,
> > Wes Williams
> > Sent from mobile phone
> >
> >> On Feb 27, 2017, at 2:07 PM, Bruce Schuchardt 
> wrote:
> >>
> >> What I like about using the existing delta-prop mechanism is it will
> also extend to client subscriptions & WAN.  It would take a lot of work to
> propagate Redis commands through those paths.
> >>
> >>> Le 2/27/2017 à 10:35 AM, Hitesh Khamesra a écrit :
> >>> The simplicity of Redis API making this problem (delta update)most
> apparent. But,  I would imagine many Geode apps has a similar use case.
> >>>
> >>> -Hitesh
> >>>
> >>>From: Michael Stolz 
> >>>   To: dev@geode.apache.org
> >>>   Sent: Monday, February 27, 2017 9:06 AM
> >>>   Subject: Re: [DISCUSS] changes to Redis implementation
> >>> It does seem like the operations will often be much smaller than
> the data
> >>> they are operating on.
> >>> It is almost the classic "move the code to the data" pattern.
> >>>
> >>> --
> >>> Mike Stolz
> >>> Principal Engineer, GemFire Product Manager
> >>> Mobile: +1-631-835-4771 <(631)%20835-4771>
> >>>
> >>> On Mon, Feb 27, 2017 at 10:51 AM, Udo Kohlmeyer  >
> >>> wrote:
> >>>
>  Delta could provide us a mechanism to replicate only what is required.
> 
>  I wonder if we could not create a simple operation replication
> framework.
>  Rather than writing a potential large amounts of code for delta, we
>  replicate only the operation.
> 
> 
> > On 2/27/17 07:18, Wes Williams wrote:
> >
> > Replicating a whole collection because of 1 change does not really
> make
> >> too much sense.<<
> > I agree but won't delta replication prevent sending the entire
> collection
> > across the wire?
> >
> > *Wes Williams | Pivotal Advisory **Data Engineer*
> > 781.606.0325 <(781)%20606-0325>
> > http://pivotal.io/big-data/pivotal-gemfire
> >
> > On Mon, Feb 27, 2017 at 10:08 AM, Udo Kohlmeyer <
> ukohlme...@pivotal.io>
> > wrote:
> >
> > I've quickly gone through the changes for the pull request.
> >> The most significant change of this pull request is that the
> collections
> >> that initially were regions are single collections (not
> distributed).
> >> That
> >> said, this is something that we've been discussing. The one thing
> that I
> >> wonder about is, what will the performance look like when the
> collections
> >> become really large? Replicating a whole collection because of 1
> change
> >> does not really make too much sense.
> >>
> >> Maybe this implementation becomes the catalyst for future
> improvements.
> >>
> >> --Udo
> >>
> >>
> >>
> >> On 2/24/17 15:25, Bruce Schuchardt wrote:
> >>
> >> Gregory Green has posted a pull request that warrants discussion. It
> >>> improves performance for Sets and Hashes by altering the storage
> format
> >>> for
> >>> these collections.  As such it will not permit a rolling upgrade,
> though
> >>> the Redis adapter is labelled "experimental" so maybe that's okay.
> >>>
> >>> https://github.com/apache/geode/pull/404
> >>>
> >>> The PR also fixes GEODE-2469, inability to handle hash keys having
> >>> colons.
> >>>
> >>> There was some discussion about altering the storage format that
> was
> >>> initiated by Hitesh.  Personally I think Gregory's changes are
> better
> >>> than
> >>> the current implementation and we should accept them, though I
> haven't
> >>> gone
> >>> through the code changes extensively.
> >>>
> >>>
> >>>
> >>>
>
>

[jira] [Commented] (GEODE-2513) Geode Native docs: rebrand to match open-source software

2017-02-27 Thread ASF subversion and git services (JIRA)


[ 
https://issues.apache.org/jira/browse/GEODE-2513?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15886395#comment-15886395
 ] 

ASF GitHub Bot commented on GEODE-2513:
---

Github user karensmolermiller closed the pull request at:

https://github.com/apache/geode-native/pull/34


> Geode Native docs: rebrand to match open-source software
> 
>
> Key: GEODE-2513
> URL: https://issues.apache.org/jira/browse/GEODE-2513
> Project: Geode
>  Issue Type: Improvement
>  Components: docs
>Reporter: Dave Barnes
>
> The newly-contributed Geode Native doc sources contain some GemFire artifacts 
> that have been purged from the open-source code. Docs should be updated to 
> match. 



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] geode-native pull request #34: GEODE-2513 Rebrand Delta Propagation section ...

2017-02-27 Thread karensmolermiller

Github user karensmolermiller closed the pull request at:

https://github.com/apache/geode-native/pull/34


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (GEODE-2513) Geode Native docs: rebrand to match open-source software


[ 
https://issues.apache.org/jira/browse/GEODE-2513?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15886388#comment-15886388
 ] 

ASF subversion and git services commented on GEODE-2513:


Commit da7cf52afab9842ae97139dc15e3f654122d3d2a in geode-native's branch 
refs/heads/develop from [~karensmolermiller]
[ https://git-wip-us.apache.org/repos/asf?p=geode-native.git;h=da7cf52 ]

GEODE-2513 Rebrand Delta Propagation section of
Geode Native documentation

- Replace outdated code example with pointer to
docs on updated QuickStart examples
- Update package names
- Cosmetic wording updates


> Geode Native docs: rebrand to match open-source software
> 
>
> Key: GEODE-2513
> URL: https://issues.apache.org/jira/browse/GEODE-2513
> Project: Geode
>  Issue Type: Improvement
>  Components: docs
>Reporter: Dave Barnes
>
> The newly-contributed Geode Native doc sources contain some GemFire artifacts 
> that have been purged from the open-source code. Docs should be updated to 
> match. 



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (GEODE-2513) Geode Native docs: rebrand to match open-source software

2017-02-27 Thread ASF subversion and git services (JIRA)


[ 
https://issues.apache.org/jira/browse/GEODE-2513?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15886390#comment-15886390
 ] 

ASF subversion and git services commented on GEODE-2513:


Commit dd5ca08860918a1dfe49cb4a81c52d581700a0ce in geode-native's branch 
refs/heads/develop from [~karensmolermiller]
[ https://git-wip-us.apache.org/repos/asf?p=geode-native.git;h=dd5ca08 ]

GEODE-2513 Remove unused delta examples section


> Geode Native docs: rebrand to match open-source software
> 
>
> Key: GEODE-2513
> URL: https://issues.apache.org/jira/browse/GEODE-2513
> Project: Geode
>  Issue Type: Improvement
>  Components: docs
>Reporter: Dave Barnes
>
> The newly-contributed Geode Native doc sources contain some GemFire artifacts 
> that have been purged from the open-source code. Docs should be updated to 
> match. 



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (GEODE-2513) Geode Native docs: rebrand to match open-source software

2017-02-27 Thread ASF subversion and git services (JIRA)


[ 
https://issues.apache.org/jira/browse/GEODE-2513?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15886389#comment-15886389
 ] 

ASF subversion and git services commented on GEODE-2513:


Commit f2090e499b02ce787f8c02953c37a71c7d6fcbae in geode-native's branch 
refs/heads/develop from [~karensmolermiller]
[ https://git-wip-us.apache.org/repos/asf?p=geode-native.git;h=f2090e4 ]

GEODE-2513 Better wording referencing the example


> Geode Native docs: rebrand to match open-source software
> 
>
> Key: GEODE-2513
> URL: https://issues.apache.org/jira/browse/GEODE-2513
> Project: Geode
>  Issue Type: Improvement
>  Components: docs
>Reporter: Dave Barnes
>
> The newly-contributed Geode Native doc sources contain some GemFire artifacts 
> that have been purged from the open-source code. Docs should be updated to 
> match. 



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

Re: [DISCUSS] changes to Redis implementation

2017-02-27 Thread Bruce Schuchardt

I think in this case it would be replacing the messages used to
replicate changes with ones that send the operation + parameters instead
of the modified entry's new value. That could be done by creating a new
subclass of BucketRegion. Stick the operation+params into the
EntryEventImpl that follows the operation through the system and in the
BucketRegion subclass pull out that data and send it instead of the
event's newValue.

Le 2/27/2017 à 11:13 AM, Real Wes a écrit :

I'm not following what a "simple operation replication framework" is and how it applies
to the Redis API. If you replicate operations, you still need to update the data at some point,
causing a synchronous replication event so as to provide HA. What is, in more detail, a
"simple operation replication framework"?

Regards,
Wes Williams
Sent from mobile phone

On Feb 27, 2017, at 2:07 PM, Bruce Schuchardt wrote:

What I like about using the existing delta-prop mechanism is it will also extend to
client subscriptions & WAN. It would take a lot of work to propagate Redis
commands through those paths.

Le 2/27/2017 à 10:35 AM, Hitesh Khamesra a écrit :
The simplicity of Redis API making this problem (delta update)most apparent.
But, I would imagine many Geode apps has a similar use case.

-Hitesh

From: Michael Stolz
To: dev@geode.apache.org
Sent: Monday, February 27, 2017 9:06 AM
Subject: Re: [DISCUSS] changes to Redis implementation
It does seem like the operations will often be much smaller than the data
they are operating on.
It is almost the classic "move the code to the data" pattern.

--
Mike Stolz
Principal Engineer, GemFire Product Manager
Mobile: +1-631-835-4771

On Mon, Feb 27, 2017 at 10:51 AM, Udo Kohlmeyer
wrote:

Delta could provide us a mechanism to replicate only what is required.

I wonder if we could not create a simple operation replication framework.
Rather than writing a potential large amounts of code for delta, we
replicate only the operation.

On 2/27/17 07:18, Wes Williams wrote:

Replicating a whole collection because of 1 change does not really make

too much sense.<<

I agree but won't delta replication prevent sending the entire collection
across the wire?

*Wes Williams | Pivotal Advisory **Data Engineer*
781.606.0325
http://pivotal.io/big-data/pivotal-gemfire

On Mon, Feb 27, 2017 at 10:08 AM, Udo Kohlmeyer
wrote:

I've quickly gone through the changes for the pull request.

The most significant change of this pull request is that the collections
that initially were regions are single collections (not distributed).
That
said, this is something that we've been discussing. The one thing that I
wonder about is, what will the performance look like when the collections
become really large? Replicating a whole collection because of 1 change
does not really make too much sense.

Maybe this implementation becomes the catalyst for future improvements.

--Udo

On 2/24/17 15:25, Bruce Schuchardt wrote:

Gregory Green has posted a pull request that warrants discussion. It

improves performance for Sets and Hashes by altering the storage format
for
these collections. As such it will not permit a rolling upgrade, though
the Redis adapter is labelled "experimental" so maybe that's okay.

https://github.com/apache/geode/pull/404

The PR also fixes GEODE-2469, inability to handle hash keys having
colons.

There was some discussion about altering the storage format that was
initiated by Hitesh. Personally I think Gregory's changes are better
than
the current implementation and we should accept them, though I haven't
gone
through the code changes extensively.

Re: [DISCUSS] changes to Redis implementation

2017-02-27 Thread Real Wes

I'm not following what a "simple operation replication framework" is and how it 
applies to the Redis API. If you replicate operations, you still need to update 
the data at some point, causing a synchronous replication event so as to 
provide HA. What is, in more detail, a "simple operation replication framework"?

Regards,
Wes Williams
Sent from mobile phone

> On Feb 27, 2017, at 2:07 PM, Bruce Schuchardt  wrote:
> 
> What I like about using the existing delta-prop mechanism is it will also 
> extend to client subscriptions & WAN.  It would take a lot of work to 
> propagate Redis commands through those paths.
> 
>> Le 2/27/2017 à 10:35 AM, Hitesh Khamesra a écrit :
>> The simplicity of Redis API making this problem (delta update)most apparent. 
>> But,  I would imagine many Geode apps has a similar use case.
>> 
>> -Hitesh
>> 
>>   From: Michael Stolz 
>>  To: dev@geode.apache.org
>>  Sent: Monday, February 27, 2017 9:06 AM
>>  Subject: Re: [DISCUSS] changes to Redis implementation
>>It does seem like the operations will often be much smaller than the data
>> they are operating on.
>> It is almost the classic "move the code to the data" pattern.
>> 
>> --
>> Mike Stolz
>> Principal Engineer, GemFire Product Manager
>> Mobile: +1-631-835-4771
>> 
>> On Mon, Feb 27, 2017 at 10:51 AM, Udo Kohlmeyer 
>> wrote:
>> 
>>> Delta could provide us a mechanism to replicate only what is required.
>>> 
>>> I wonder if we could not create a simple operation replication framework.
>>> Rather than writing a potential large amounts of code for delta, we
>>> replicate only the operation.
>>> 
>>> 
 On 2/27/17 07:18, Wes Williams wrote:
 
 Replicating a whole collection because of 1 change does not really make
> too much sense.<<
 I agree but won't delta replication prevent sending the entire collection
 across the wire?
 
 *Wes Williams | Pivotal Advisory **Data Engineer*
 781.606.0325
 http://pivotal.io/big-data/pivotal-gemfire
 
 On Mon, Feb 27, 2017 at 10:08 AM, Udo Kohlmeyer 
 wrote:
 
 I've quickly gone through the changes for the pull request.
> The most significant change of this pull request is that the collections
> that initially were regions are single collections (not distributed).
> That
> said, this is something that we've been discussing. The one thing that I
> wonder about is, what will the performance look like when the collections
> become really large? Replicating a whole collection because of 1 change
> does not really make too much sense.
> 
> Maybe this implementation becomes the catalyst for future improvements.
> 
> --Udo
> 
> 
> 
> On 2/24/17 15:25, Bruce Schuchardt wrote:
> 
> Gregory Green has posted a pull request that warrants discussion. It
>> improves performance for Sets and Hashes by altering the storage format
>> for
>> these collections.  As such it will not permit a rolling upgrade, though
>> the Redis adapter is labelled "experimental" so maybe that's okay.
>> 
>> https://github.com/apache/geode/pull/404
>> 
>> The PR also fixes GEODE-2469, inability to handle hash keys having
>> colons.
>> 
>> There was some discussion about altering the storage format that was
>> initiated by Hitesh.  Personally I think Gregory's changes are better
>> than
>> the current implementation and we should accept them, though I haven't
>> gone
>> through the code changes extensively.
>> 
>> 
>> 
>> 
>>
>

Re: [DISCUSS] changes to Redis implementation

2017-02-27 Thread Bruce Schuchardt

What I like about using the existing delta-prop mechanism is it will 
also extend to client subscriptions & WAN.  It would take a lot of work 
to propagate Redis commands through those paths.


Le 2/27/2017 à 10:35 AM, Hitesh Khamesra a écrit :

The simplicity of Redis API making this problem (delta update)most apparent. 
But,  I would imagine many Geode apps has a similar use case.

-Hitesh

   From: Michael Stolz 
  To: dev@geode.apache.org
  Sent: Monday, February 27, 2017 9:06 AM
  Subject: Re: [DISCUSS] changes to Redis implementation

It does seem like the operations will often be much smaller than the data

they are operating on.
It is almost the classic "move the code to the data" pattern.

--
Mike Stolz
Principal Engineer, GemFire Product Manager
Mobile: +1-631-835-4771

On Mon, Feb 27, 2017 at 10:51 AM, Udo Kohlmeyer 
wrote:


Delta could provide us a mechanism to replicate only what is required.

I wonder if we could not create a simple operation replication framework.
Rather than writing a potential large amounts of code for delta, we
replicate only the operation.


On 2/27/17 07:18, Wes Williams wrote:


Replicating a whole collection because of 1 change does not really make

too much sense.<<

I agree but won't delta replication prevent sending the entire collection
across the wire?

*Wes Williams | Pivotal Advisory **Data Engineer*
781.606.0325
http://pivotal.io/big-data/pivotal-gemfire

On Mon, Feb 27, 2017 at 10:08 AM, Udo Kohlmeyer 
wrote:

I've quickly gone through the changes for the pull request.

The most significant change of this pull request is that the collections
that initially were regions are single collections (not distributed).
That
said, this is something that we've been discussing. The one thing that I
wonder about is, what will the performance look like when the collections
become really large? Replicating a whole collection because of 1 change
does not really make too much sense.

Maybe this implementation becomes the catalyst for future improvements.

--Udo



On 2/24/17 15:25, Bruce Schuchardt wrote:

Gregory Green has posted a pull request that warrants discussion. It

improves performance for Sets and Hashes by altering the storage format
for
these collections.  As such it will not permit a rolling upgrade, though
the Redis adapter is labelled "experimental" so maybe that's okay.

https://github.com/apache/geode/pull/404

The PR also fixes GEODE-2469, inability to handle hash keys having
colons.

There was some discussion about altering the storage format that was
initiated by Hitesh.  Personally I think Gregory's changes are better
than
the current implementation and we should accept them, though I haven't
gone
through the code changes extensively.

[jira] [Commented] (GEODE-2494) Replace SpinLock class with C++11 style BasicLockable class, spinlock_mutex.


[ 
https://issues.apache.org/jira/browse/GEODE-2494?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15886303#comment-15886303
 ] 

ASF GitHub Bot commented on GEODE-2494:
---

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

https://github.com/apache/geode-native/pull/36#discussion_r103281152
  
--- Diff: src/tests/cpp/security/Security.cpp ---
@@ -41,16 +41,18 @@
 
 #include "security/CredentialGenerator.hpp"
 
-namespace FwkSecurity {
+#include 
+#include 
+
+namespace apache {
+namespace geode {
+namespace client {
+namespace testframework {
+namespace security {
--- End diff --

Could these constants be moved into an anonymous namespace?


> Replace SpinLock class with C++11 style BasicLockable class, spinlock_mutex.
> 
>
> Key: GEODE-2494
> URL: https://issues.apache.org/jira/browse/GEODE-2494
> Project: Geode
>  Issue Type: Sub-task
>  Components: native client
>Reporter: Jacob S. Barrett
>Assignee: Jacob S. Barrett
>
> Replace {{SpinLock}} class with C++11 style 
> {{[BasicLockable|http://en.cppreference.com/w/cpp/concept/BasicLockable]}} 
> class, {{spinlock_mutex}}. You can find several public domain examples of how 
> to implement a {{spinlock_mutex}} that can be used with 
> {{[std::lock_guard|http://en.cppreference.com/w/cpp/thread/lock_guard]}}.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] geode-native pull request #36: GEODE-2494: Replace SpinLock with spinlock_mu...

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

https://github.com/apache/geode-native/pull/36#discussion_r103281152
  
--- Diff: src/tests/cpp/security/Security.cpp ---
@@ -41,16 +41,18 @@
 
 #include "security/CredentialGenerator.hpp"
 
-namespace FwkSecurity {
+#include 
+#include 
+
+namespace apache {
+namespace geode {
+namespace client {
+namespace testframework {
+namespace security {
--- End diff --

Could these constants be moved into an anonymous namespace?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (GEODE-2494) Replace SpinLock class with C++11 style BasicLockable class, spinlock_mutex.


[ 
https://issues.apache.org/jira/browse/GEODE-2494?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15886294#comment-15886294
 ] 

ASF GitHub Bot commented on GEODE-2494:
---

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

https://github.com/apache/geode-native/pull/36#discussion_r103280113
  
--- Diff: src/cppcache/src/LRUEntriesMap.cpp ---
@@ -20,6 +20,9 @@
 #include "MapSegment.hpp"
 #include "CacheImpl.hpp"
 
+#include 
+#include "util/concurrent/spinlock_mutex.hpp"
--- End diff --

Similar to a previous comment from @dgkimura.


> Replace SpinLock class with C++11 style BasicLockable class, spinlock_mutex.
> 
>
> Key: GEODE-2494
> URL: https://issues.apache.org/jira/browse/GEODE-2494
> Project: Geode
>  Issue Type: Sub-task
>  Components: native client
>Reporter: Jacob S. Barrett
>Assignee: Jacob S. Barrett
>
> Replace {{SpinLock}} class with C++11 style 
> {{[BasicLockable|http://en.cppreference.com/w/cpp/concept/BasicLockable]}} 
> class, {{spinlock_mutex}}. You can find several public domain examples of how 
> to implement a {{spinlock_mutex}} that can be used with 
> {{[std::lock_guard|http://en.cppreference.com/w/cpp/thread/lock_guard]}}.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] geode-native pull request #36: GEODE-2494: Replace SpinLock with spinlock_mu...

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

https://github.com/apache/geode-native/pull/36#discussion_r103280043
  
--- Diff: src/cppcache/src/LRUEntriesMap.cpp ---
@@ -297,7 +295,7 @@ GfErrType LRUEntriesMap::put(const CacheableKeyPtr& key,
   }
 }
 // SpinLock& lock = segmentRPtr->getSpinLock();
-// SpinLockGuard mapGuard( lock );
+// std::lock_guard mapGuard( lock );
--- End diff --

Remove commented out code?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (GEODE-2494) Replace SpinLock class with C++11 style BasicLockable class, spinlock_mutex.


[ 
https://issues.apache.org/jira/browse/GEODE-2494?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15886295#comment-15886295
 ] 

ASF GitHub Bot commented on GEODE-2494:
---

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

https://github.com/apache/geode-native/pull/36#discussion_r103280043
  
--- Diff: src/cppcache/src/LRUEntriesMap.cpp ---
@@ -297,7 +295,7 @@ GfErrType LRUEntriesMap::put(const CacheableKeyPtr& key,
   }
 }
 // SpinLock& lock = segmentRPtr->getSpinLock();
-// SpinLockGuard mapGuard( lock );
+// std::lock_guard mapGuard( lock );
--- End diff --

Remove commented out code?


> Replace SpinLock class with C++11 style BasicLockable class, spinlock_mutex.
> 
>
> Key: GEODE-2494
> URL: https://issues.apache.org/jira/browse/GEODE-2494
> Project: Geode
>  Issue Type: Sub-task
>  Components: native client
>Reporter: Jacob S. Barrett
>Assignee: Jacob S. Barrett
>
> Replace {{SpinLock}} class with C++11 style 
> {{[BasicLockable|http://en.cppreference.com/w/cpp/concept/BasicLockable]}} 
> class, {{spinlock_mutex}}. You can find several public domain examples of how 
> to implement a {{spinlock_mutex}} that can be used with 
> {{[std::lock_guard|http://en.cppreference.com/w/cpp/thread/lock_guard]}}.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] geode-native pull request #36: GEODE-2494: Replace SpinLock with spinlock_mu...

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

https://github.com/apache/geode-native/pull/36#discussion_r103280113
  
--- Diff: src/cppcache/src/LRUEntriesMap.cpp ---
@@ -20,6 +20,9 @@
 #include "MapSegment.hpp"
 #include "CacheImpl.hpp"
 
+#include 
+#include "util/concurrent/spinlock_mutex.hpp"
--- End diff --

Similar to a previous comment from @dgkimura.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (GEODE-2494) Replace SpinLock class with C++11 style BasicLockable class, spinlock_mutex.


[ 
https://issues.apache.org/jira/browse/GEODE-2494?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15886292#comment-15886292
 ] 

ASF GitHub Bot commented on GEODE-2494:
---

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

https://github.com/apache/geode-native/pull/36#discussion_r103279475
  
--- Diff: src/cppcache/src/MapSegment.cpp ---
@@ -316,16 +320,16 @@ GfErrType MapSegment::remove(const CacheableKeyPtr& 
key, CacheablePtr& oldValue,
  MapEntryImplPtr& me, int updateCount,
  VersionTagPtr versionTag, bool afterRemote,
  bool& isEntryFound) {
-  //  _GF_GUARD_SEGMENT;
+  //  std::lock_guard lk(m_spinlock);
--- End diff --

Since the code is commented out, should it just be deleted?


> Replace SpinLock class with C++11 style BasicLockable class, spinlock_mutex.
> 
>
> Key: GEODE-2494
> URL: https://issues.apache.org/jira/browse/GEODE-2494
> Project: Geode
>  Issue Type: Sub-task
>  Components: native client
>Reporter: Jacob S. Barrett
>Assignee: Jacob S. Barrett
>
> Replace {{SpinLock}} class with C++11 style 
> {{[BasicLockable|http://en.cppreference.com/w/cpp/concept/BasicLockable]}} 
> class, {{spinlock_mutex}}. You can find several public domain examples of how 
> to implement a {{spinlock_mutex}} that can be used with 
> {{[std::lock_guard|http://en.cppreference.com/w/cpp/thread/lock_guard]}}.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (GEODE-2494) Replace SpinLock class with C++11 style BasicLockable class, spinlock_mutex.


[ 
https://issues.apache.org/jira/browse/GEODE-2494?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15886291#comment-15886291
 ] 

ASF GitHub Bot commented on GEODE-2494:
---

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

https://github.com/apache/geode-native/pull/36#discussion_r103279753
  
--- Diff: src/cppcache/src/MapSegment.hpp ---
@@ -164,9 +166,9 @@ class CPPCACHE_EXPORT MapSegment {
 m_entryFactory->newMapEntry(key, newEntry);
 newEntry->setValueI(newValue);
 if (m_concurrencyChecksEnabled) {
-  if (versionTag != NULLPTR && versionTag.ptr() != NULL) {
+  if (versionTag != NULLPTR && versionTag.ptr() != nullptr) {
--- End diff --

It looks like `nullptr` is being used, not `NULLPTR`, no?


> Replace SpinLock class with C++11 style BasicLockable class, spinlock_mutex.
> 
>
> Key: GEODE-2494
> URL: https://issues.apache.org/jira/browse/GEODE-2494
> Project: Geode
>  Issue Type: Sub-task
>  Components: native client
>Reporter: Jacob S. Barrett
>Assignee: Jacob S. Barrett
>
> Replace {{SpinLock}} class with C++11 style 
> {{[BasicLockable|http://en.cppreference.com/w/cpp/concept/BasicLockable]}} 
> class, {{spinlock_mutex}}. You can find several public domain examples of how 
> to implement a {{spinlock_mutex}} that can be used with 
> {{[std::lock_guard|http://en.cppreference.com/w/cpp/thread/lock_guard]}}.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] geode-native pull request #36: GEODE-2494: Replace SpinLock with spinlock_mu...

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

https://github.com/apache/geode-native/pull/36#discussion_r103279753
  
--- Diff: src/cppcache/src/MapSegment.hpp ---
@@ -164,9 +166,9 @@ class CPPCACHE_EXPORT MapSegment {
 m_entryFactory->newMapEntry(key, newEntry);
 newEntry->setValueI(newValue);
 if (m_concurrencyChecksEnabled) {
-  if (versionTag != NULLPTR && versionTag.ptr() != NULL) {
+  if (versionTag != NULLPTR && versionTag.ptr() != nullptr) {
--- End diff --

It looks like `nullptr` is being used, not `NULLPTR`, no?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode-native pull request #36: GEODE-2494: Replace SpinLock with spinlock_mu...

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

https://github.com/apache/geode-native/pull/36#discussion_r103279475
  
--- Diff: src/cppcache/src/MapSegment.cpp ---
@@ -316,16 +320,16 @@ GfErrType MapSegment::remove(const CacheableKeyPtr& 
key, CacheablePtr& oldValue,
  MapEntryImplPtr& me, int updateCount,
  VersionTagPtr versionTag, bool afterRemote,
  bool& isEntryFound) {
-  //  _GF_GUARD_SEGMENT;
+  //  std::lock_guard lk(m_spinlock);
--- End diff --

Since the code is commented out, should it just be deleted?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (GEODE-2494) Replace SpinLock class with C++11 style BasicLockable class, spinlock_mutex.


[ 
https://issues.apache.org/jira/browse/GEODE-2494?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15886285#comment-15886285
 ] 

ASF GitHub Bot commented on GEODE-2494:
---

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

https://github.com/apache/geode-native/pull/36#discussion_r103278690
  
--- Diff: src/cppcache/src/LRUList.cpp ---
@@ -15,12 +15,18 @@
  * limitations under the License.
  */
 #include "LRUList.hpp"
-#include "SpinLock.hpp"
+#include "util/concurrent/spinlock_mutex.hpp"
 
-using namespace apache::geode::client;
+#include 
 
-#define LOCK_HEAD SpinLockGuard headLockGuard(m_headLock)
-#define LOCK_TAIL SpinLockGuard tailLockGuard(m_tailLock)
--- End diff --

Good to see the removal of more macros.


> Replace SpinLock class with C++11 style BasicLockable class, spinlock_mutex.
> 
>
> Key: GEODE-2494
> URL: https://issues.apache.org/jira/browse/GEODE-2494
> Project: Geode
>  Issue Type: Sub-task
>  Components: native client
>Reporter: Jacob S. Barrett
>Assignee: Jacob S. Barrett
>
> Replace {{SpinLock}} class with C++11 style 
> {{[BasicLockable|http://en.cppreference.com/w/cpp/concept/BasicLockable]}} 
> class, {{spinlock_mutex}}. You can find several public domain examples of how 
> to implement a {{spinlock_mutex}} that can be used with 
> {{[std::lock_guard|http://en.cppreference.com/w/cpp/thread/lock_guard]}}.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (GEODE-2494) Replace SpinLock class with C++11 style BasicLockable class, spinlock_mutex.


[ 
https://issues.apache.org/jira/browse/GEODE-2494?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15886286#comment-15886286
 ] 

ASF GitHub Bot commented on GEODE-2494:
---

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

https://github.com/apache/geode-native/pull/36#discussion_r103279163
  
--- Diff: src/cppcache/src/LRUList.hpp ---
@@ -20,32 +20,32 @@
  * limitations under the License.
  */
 
+#include 
+
 #include 
 #include 
-#include "SpinLock.hpp"
+
+#include "util/concurrent/spinlock_mutex.hpp"
 
 namespace apache {
 namespace geode {
 namespace client {
+
 // Bit mask for recently used
-#define RECENTLY_USED_BITS 1ul
+#define RECENTLY_USED_BITS 1u
 // Bit mask for evicted
-#define EVICTED_BITS 2ul
+#define EVICTED_BITS 2u
--- End diff --

Could these be const variables in an anonymous namespace instead?


> Replace SpinLock class with C++11 style BasicLockable class, spinlock_mutex.
> 
>
> Key: GEODE-2494
> URL: https://issues.apache.org/jira/browse/GEODE-2494
> Project: Geode
>  Issue Type: Sub-task
>  Components: native client
>Reporter: Jacob S. Barrett
>Assignee: Jacob S. Barrett
>
> Replace {{SpinLock}} class with C++11 style 
> {{[BasicLockable|http://en.cppreference.com/w/cpp/concept/BasicLockable]}} 
> class, {{spinlock_mutex}}. You can find several public domain examples of how 
> to implement a {{spinlock_mutex}} that can be used with 
> {{[std::lock_guard|http://en.cppreference.com/w/cpp/thread/lock_guard]}}.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[GitHub] geode-native pull request #36: GEODE-2494: Replace SpinLock with spinlock_mu...

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

https://github.com/apache/geode-native/pull/36#discussion_r103278690
  
--- Diff: src/cppcache/src/LRUList.cpp ---
@@ -15,12 +15,18 @@
  * limitations under the License.
  */
 #include "LRUList.hpp"
-#include "SpinLock.hpp"
+#include "util/concurrent/spinlock_mutex.hpp"
 
-using namespace apache::geode::client;
+#include 
 
-#define LOCK_HEAD SpinLockGuard headLockGuard(m_headLock)
-#define LOCK_TAIL SpinLockGuard tailLockGuard(m_tailLock)
--- End diff --

Good to see the removal of more macros.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode-native pull request #36: GEODE-2494: Replace SpinLock with spinlock_mu...