Region data corruption due to missing PdxTypes

[Mario Salazar de Torres]

Hi everyone,

While debugging some coredumps in the native client related to PdxTypeRegistry 
cleanup, I tried to reproduce the scenario with the Java client API to see how 
it was handled.
Thing is I've noticed that this scenario in the Java client might lead to Geode 
storing a corrupted entry, meaning that queries won't work on those regions 
containing corrupted entries.
And with corrupted entries, I refer to entries using a missing PdxType. The 
scenario involves a cluster restart. It's described below:

  1.  Start a cluster with 1 locator and 3 servers, and persistence is disabled 
for PdxTypes.
  2.  Setup a region called "test-region" with persistence disabled. It doesn't 
mind whether is replicated or partitioned.
  3.  In the client, instantiate the client region with PROXY region shortcut 
and establish the connection toward the cluster.
  4.  In the client, create a PdxInstance and put in into the "test-region" 
with key "test".
  5.  In the client, get the entry which key is "test", which turns out to be 
the PdxInstance inserted in step 4.
  6.  At this point, cluster is restarted, meaning that all the data is lost, 
included PdxTypes.
  7.  In the client, the PdxInstance obtained in step 5 is put into 
"test-region" with key "test2"
  8.  In the client, the following query is executed: "SELECT * FROM 
/test-region WHERE value = -1".
Such query fails with the message "Unknown pdx type=" and it won't 
work until the corrupted entry is removed.

Also, the above scenario could be solved by enabling persistence for PdxTypes, 
but if you have an unrecoverable issue in your cluster and you need to spin up 
a backup,
it could happen that PdxInstance's PdxType obtained step 5 is not present in 
the backup, leading to the entry being inserted but, yet again, the PdxType 
being missing.

It's worth mentioning that in the native client, this scenario currently 
results in a coredump, but no data corruption,
given that after losing the connection towards the cluster PdxTypeRegistry is 
cleaned up and PdxTypes are obtained with its ID, rather than directly using 
the object.

My question here are:

  *   Have you seen this issue before?
  *   Is there a way to verify that PdxTypes are present in the cluster before 
writing an entry which holds some PdxInstances?

Thanks,
Mario.

Re: DISCUSSION: Geode Native C++ Style and Formatting Guide

[Mario Salazar de Torres]

Hi everyone,

Regarding Blake's comment on moving towards C++17 I completely agree.
I think almost every compiler supports it now and given the latest changes on 
WoW regarding ABI,
even if there were some ABI break (which is not the case from C++11 to C++17), 
everything should be fine.

I'd really love to see C++20 as the standard for the project as it has several 
cool features, but sadly I must admit is too recent to be adopted.

Thanks,
Mario.

From: Blake Bender 
Sent: Monday, May 3, 2021 8:23 PM
To: dev@geode.apache.org 
Subject: RE: DISCUSSION: Geode Native C++ Style and Formatting Guide

My $0.02 on these:

Things I'd like to see us conform to Google style on:
* I'd be happy to move to C++ 17
* Would also be happy to remove forward declarations.  "I'm not a critic, but I 
know what I hate," as it were, and I hate forward declarations.
* I would also be happy with an 80-character line limit, though I don't feel 
strongly about it.  100 may be consistent with Geode, but it still feels 
arbitrary to me.
* I would be very pleased to remove all the macros from our code.  I've been 
bitten more than once in the past while debugging or refactoring our code, 
because of ill-formed macros.

Google things I disagree with:
* I don't like exceptions, but I don't even want to think about the amount of 
effort required to remove them from the codebase is, IMO, unreasonably high.  
Keep the exceptions, most of the time they're used pretty judiciously.
* I really, really, *really* (really?  Yes, really!) hate anything resembling 
Hungarian prefix notation, and have permanent scars from decades of reading it 
in Windows code.  Please don't ask me to put a random 'k' in from of my enums - 
ick.

One other note: in the past, we've had conversations about "style only" pull 
requests to fix some of these things, and the guidance we ended up with has 
been to only fix this sort of thing while you're in the code working on a fix 
or a feature.  I, for one, would welcome some PRs that just, say, renamed a ton 
of member variables to replace "m_" prefix with a simple trailing "_", perhaps 
fixed some of the more egregious and weird abbreviations, etc.  My preference 
for bug fixes and feature work is that all of the code changes be focused on 
stuff that's relevant to the fix/feature, and mixing it with random style guide 
refactoring, I feel, muddies the waters for future maintainers.

Thanks,

Blake

-Original Message-
From: Jacob Barrett 
Sent: Saturday, May 1, 2021 9:21 AM
To: dev@geode.apache.org
Subject: Re: DISCUSSION: Geode Native C++ Style and Formatting Guide

Great call outs!

> On May 1, 2021, at 7:57 AM, Mario Salazar de Torres 
>  wrote:
>
>  1.  Member variables names as of Google style guide requires a '_' char to 
> be added at the end so it can be identified. Should we also adopt that?
> For example, imagine you have a region mutex, so, should we name it as 
> 'regionMutex_' ?

I didn’t mention this one out in my review of differences because we are 
following it but I suppose with the combination of the camelCase difference we 
should probably call it out more specifically. Perhaps in our documentation we 
should show examples of both local and member variables. Do you think that will 
make it more clear?

>  2.  Also, I would like to point out that macros are dis-recommended but 
> every C++ committee member I know.
> What do you think about adding a notice saying: "Macros should be avoided and 
> only used when there is no alternative”?

I think that is called out in various ways in a few places in the Google guide 
but I am more than happy for us to include strong or clearer language around 
this. Between constexpr and templates there are very cases for macros anymore.
We mostly use macros only to handle non-standard attributes. When we move to 
C++17 a lot of these will go away.

Thanks,
Jake

Re: DISCUSSION: Geode Native C++ Style and Formatting Guide

[Jacob Barrett]

Mario,

Let’s start a separate discussion for going to C++17 on a minor release. Can 
you lay out the pros and cons to kick it off?

> On May 4, 2021, at 7:31 AM, Mario Salazar de Torres 
>  wrote:
> 
> Hi everyone,
> 
> Regarding Blake's comment on moving towards C++17 I completely agree.
> I think almost every compiler supports it now and given the latest changes on 
> WoW regarding ABI,
> even if there were some ABI break (which is not the case from C++11 to 
> C++17), everything should be fine.
> 
> I'd really love to see C++20 as the standard for the project as it has 
> several cool features, but sadly I must admit is too recent to be adopted.
> 
> Thanks,
> Mario.
> 
> From: Blake Bender 
> Sent: Monday, May 3, 2021 8:23 PM
> To: dev@geode.apache.org 
> Subject: RE: DISCUSSION: Geode Native C++ Style and Formatting Guide
> 
> My $0.02 on these:
> 
> Things I'd like to see us conform to Google style on:
> * I'd be happy to move to C++ 17
> * Would also be happy to remove forward declarations.  "I'm not a critic, but 
> I know what I hate," as it were, and I hate forward declarations.
> * I would also be happy with an 80-character line limit, though I don't feel 
> strongly about it.  100 may be consistent with Geode, but it still feels 
> arbitrary to me.
> * I would be very pleased to remove all the macros from our code.  I've been 
> bitten more than once in the past while debugging or refactoring our code, 
> because of ill-formed macros.
> 
> Google things I disagree with:
> * I don't like exceptions, but I don't even want to think about the amount of 
> effort required to remove them from the codebase is, IMO, unreasonably high.  
> Keep the exceptions, most of the time they're used pretty judiciously.
> * I really, really, *really* (really?  Yes, really!) hate anything resembling 
> Hungarian prefix notation, and have permanent scars from decades of reading 
> it in Windows code.  Please don't ask me to put a random 'k' in from of my 
> enums - ick.
> 
> One other note: in the past, we've had conversations about "style only" pull 
> requests to fix some of these things, and the guidance we ended up with has 
> been to only fix this sort of thing while you're in the code working on a fix 
> or a feature.  I, for one, would welcome some PRs that just, say, renamed a 
> ton of member variables to replace "m_" prefix with a simple trailing "_", 
> perhaps fixed some of the more egregious and weird abbreviations, etc.  My 
> preference for bug fixes and feature work is that all of the code changes be 
> focused on stuff that's relevant to the fix/feature, and mixing it with 
> random style guide refactoring, I feel, muddies the waters for future 
> maintainers.
> 
> Thanks,
> 
> Blake
> 
> -Original Message-
> From: Jacob Barrett 
> Sent: Saturday, May 1, 2021 9:21 AM
> To: dev@geode.apache.org
> Subject: Re: DISCUSSION: Geode Native C++ Style and Formatting Guide
> 
> Great call outs!
> 
>> On May 1, 2021, at 7:57 AM, Mario Salazar de Torres 
>>  wrote:
>> 
>> 1.  Member variables names as of Google style guide requires a '_' char to 
>> be added at the end so it can be identified. Should we also adopt that?
>> For example, imagine you have a region mutex, so, should we name it as 
>> 'regionMutex_' ?
> 
> I didn’t mention this one out in my review of differences because we are 
> following it but I suppose with the combination of the camelCase difference 
> we should probably call it out more specifically. Perhaps in our 
> documentation we should show examples of both local and member variables. Do 
> you think that will make it more clear?
> 
>> 2.  Also, I would like to point out that macros are dis-recommended but 
>> every C++ committee member I know.
>> What do you think about adding a notice saying: "Macros should be avoided 
>> and only used when there is no alternative”?
> 
> I think that is called out in various ways in a few places in the Google 
> guide but I am more than happy for us to include strong or clearer language 
> around this. Between constexpr and templates there are very cases for macros 
> anymore.
> We mostly use macros only to handle non-standard attributes. When we move to 
> C++17 a lot of these will go away.
> 
> Thanks,
> Jake
> 
> 
> 

Re: DISCUSSION: Geode Native C++ Style and Formatting Guide

[Mario Salazar de Torres]

Sure, gladly 🙂. Let me put up the info and I will open the discussion.
And sorry for polluting this topic regarding Style and Formatting :S

Thanks,
Mario.

From: Jacob Barrett 
Sent: Tuesday, May 4, 2021 4:42 PM
To: dev@geode.apache.org 
Subject: Re: DISCUSSION: Geode Native C++ Style and Formatting Guide

Mario,

Let’s start a separate discussion for going to C++17 on a minor release. Can 
you lay out the pros and cons to kick it off?

> On May 4, 2021, at 7:31 AM, Mario Salazar de Torres 
>  wrote:
>
> Hi everyone,
>
> Regarding Blake's comment on moving towards C++17 I completely agree.
> I think almost every compiler supports it now and given the latest changes on 
> WoW regarding ABI,
> even if there were some ABI break (which is not the case from C++11 to 
> C++17), everything should be fine.
>
> I'd really love to see C++20 as the standard for the project as it has 
> several cool features, but sadly I must admit is too recent to be adopted.
>
> Thanks,
> Mario.
> 
> From: Blake Bender 
> Sent: Monday, May 3, 2021 8:23 PM
> To: dev@geode.apache.org 
> Subject: RE: DISCUSSION: Geode Native C++ Style and Formatting Guide
>
> My $0.02 on these:
>
> Things I'd like to see us conform to Google style on:
> * I'd be happy to move to C++ 17
> * Would also be happy to remove forward declarations.  "I'm not a critic, but 
> I know what I hate," as it were, and I hate forward declarations.
> * I would also be happy with an 80-character line limit, though I don't feel 
> strongly about it.  100 may be consistent with Geode, but it still feels 
> arbitrary to me.
> * I would be very pleased to remove all the macros from our code.  I've been 
> bitten more than once in the past while debugging or refactoring our code, 
> because of ill-formed macros.
>
> Google things I disagree with:
> * I don't like exceptions, but I don't even want to think about the amount of 
> effort required to remove them from the codebase is, IMO, unreasonably high.  
> Keep the exceptions, most of the time they're used pretty judiciously.
> * I really, really, *really* (really?  Yes, really!) hate anything resembling 
> Hungarian prefix notation, and have permanent scars from decades of reading 
> it in Windows code.  Please don't ask me to put a random 'k' in from of my 
> enums - ick.
>
> One other note: in the past, we've had conversations about "style only" pull 
> requests to fix some of these things, and the guidance we ended up with has 
> been to only fix this sort of thing while you're in the code working on a fix 
> or a feature.  I, for one, would welcome some PRs that just, say, renamed a 
> ton of member variables to replace "m_" prefix with a simple trailing "_", 
> perhaps fixed some of the more egregious and weird abbreviations, etc.  My 
> preference for bug fixes and feature work is that all of the code changes be 
> focused on stuff that's relevant to the fix/feature, and mixing it with 
> random style guide refactoring, I feel, muddies the waters for future 
> maintainers.
>
> Thanks,
>
> Blake
>
> -Original Message-
> From: Jacob Barrett 
> Sent: Saturday, May 1, 2021 9:21 AM
> To: dev@geode.apache.org
> Subject: Re: DISCUSSION: Geode Native C++ Style and Formatting Guide
>
> Great call outs!
>
>> On May 1, 2021, at 7:57 AM, Mario Salazar de Torres 
>>  wrote:
>>
>> 1.  Member variables names as of Google style guide requires a '_' char to 
>> be added at the end so it can be identified. Should we also adopt that?
>> For example, imagine you have a region mutex, so, should we name it as 
>> 'regionMutex_' ?
>
> I didn’t mention this one out in my review of differences because we are 
> following it but I suppose with the combination of the camelCase difference 
> we should probably call it out more specifically. Perhaps in our 
> documentation we should show examples of both local and member variables. Do 
> you think that will make it more clear?
>
>> 2.  Also, I would like to point out that macros are dis-recommended but 
>> every C++ committee member I know.
>> What do you think about adding a notice saying: "Macros should be avoided 
>> and only used when there is no alternative”?
>
> I think that is called out in various ways in a few places in the Google 
> guide but I am more than happy for us to include strong or clearer language 
> around this. Between constexpr and templates there are very cases for macros 
> anymore.
> We mostly use macros only to handle non-standard attributes. When we move to 
> C++17 a lot of these will go away.
>
> Thanks,
> Jake
>
>
>

Re: DISCUSSION: Geode Native C++ Style and Formatting Guide

[Jacob Barrett]

> On May 4, 2021, at 7:59 AM, Mario Salazar de Torres 
>  wrote:
> 
> Sure, gladly 🙂. Let me put up the info and I will open the discussion.
> And sorry for polluting this topic regarding Style and Formatting :S

No apologies! I just think this topic has a much bigger scope than documenting 
our deviations from the style. It has larger ramifications than say changing 
variable names. It is as discussion I am excited to help champion too.

Thanks,
Jake

DISCUSSION: Geode Native C++ 17 adoption

[Mario Salazar de Torres]

Hi everyone,

Currently Geode Native uses C++11 standard. It has been quite some time since 
the standard was released and as of today the latest standard is C++20.
As part of another discussion, some users in the community were wondering if 
it's the time to switch to C++17 in the Geode Native project.

So, I am putting a list of pros and cons:

Pros:

  *   Several new features added:
 *   C++14 features: 
https://en.cppreference.com/w/cpp/14#New_language_features
 *   C++17 features: 
https://en.cppreference.com/w/cpp/17#New_language_features
  *   Some of the interesting features are:
 *   Function return type deduction.
 *   Improved constexpr functions.
 *   Variable templates.
 - Generic lambdas.
 - Lambda capture expressions.
 - [[deprecated]]
 - Shared mutexes/locks.
 - std::make_unique
C++17 - 
cppreference.com
This page was last modified on 20 October 2020, at 04:39. This page has been 
accessed 106,431 times. Privacy policy; About cppreference.com; Disclaimers
en.cppreference.com



C++14 - 
cppreference.com
New language features . variable templates; generic lambdas; lambda 
init-capture new/delete elision relaxed restrictions on constexpr functions; 
binary literals
en.cppreference.com


Re: DISCUSSION: Geode Native C++ 17 adoption

[Mario Salazar de Torres]

Hi everyone,

Sorry for the previous email, I did send it before finishing it by mistake.

Currently Geode Native uses C++11 standard. It has been quite some time since 
the standard was released and as of today the latest standard is C++20.
As part of another discussion, some users in the community were wondering if 
it's the time to switch to C++17 in the Geode Native project.

So, I am putting a list of pros and cons:

Pros:

  *   Several new features added:
 *   C++14 features: 
https://en.cppreference.com/w/cpp/14#New_language_features
 *   C++17 features: 
https://en.cppreference.com/w/cpp/17#New_language_features
  *   Some of the interesting features are:
 *   Function return type deduction.
 *   Improved constexpr functions.
 *   Variable templates.
 *   Generic lambdas.
 *   Lambda capture expressions.
 *   [[deprecated]]
 *   Shared mutexes/locks.
 *   std::make_unique
 *   Nested namespace definitions.
 *   Structured bindings.
 *   variant.
 *   any.
 *   optional.

Cons:
  *   Some users might have older compilers which does not implement all C++ 17 
features.

Thanks,
Mario.


From: Mario Salazar de Torres 
Sent: Tuesday, May 4, 2021 7:34 PM
To: dev@geode.apache.org 
Subject: DISCUSSION: Geode Native C++ 17 adoption

Hi everyone,

Currently Geode Native uses C++11 standard. It has been quite some time since 
the standard was released and as of today the latest standard is C++20.
As part of another discussion, some users in the community were wondering if 
it's the time to switch to C++17 in the Geode Native project.

So, I am putting a list of pros and cons:

Pros:

  *   Several new features added:
 *   C++14 features: 
https://en.cppreference.com/w/cpp/14#New_language_features
 *   C++17 features: 
https://en.cppreference.com/w/cpp/17#New_language_features
  *   Some of the interesting features are:
 *   Function return type deduction.
 *   Improved constexpr functions.
 *   Variable templates.
 - Generic lambdas.
 - Lambda capture expressions.
 - [[deprecated]]
 - Shared mutexes/locks.
 - std::make_unique
C++17 - 
cppreference.com
This page was last modified on 20 October 2020, at 04:39. This page has been 
accessed 106,431 times. Privacy policy; About cppreference.com; Disclaimers
en.cppreference.com



C++14 - 
cppreference.com
New language features . variable templates; generic lambdas; lambda 
init-capture new/delete elision relaxed restrictions on constexpr functions; 
binary literals
en.cppreference.com


Re: DISCUSSION: Geode Native C++ 17 adoption

[Alberto Gomez]

Hi,

Here come my two cents.

To me, upgrading to C++17 is a no brainer given that C++11 is quite old and 
C++17 has lots of new features, performance improvements and bug fixes.

The only thing that could prevent us from doing so is having lots of users that 
are running the native client in a platform that does not have a C++17 
compiler. Which leads me to the question: should we move this discussion to the 
user mailing list?

Alberto

From: Mario Salazar de Torres 
Sent: Tuesday, May 4, 2021 7:45 PM
To: dev@geode.apache.org 
Subject: Re: DISCUSSION: Geode Native C++ 17 adoption

Hi everyone,

Sorry for the previous email, I did send it before finishing it by mistake.

Currently Geode Native uses C++11 standard. It has been quite some time since 
the standard was released and as of today the latest standard is C++20.
As part of another discussion, some users in the community were wondering if 
it's the time to switch to C++17 in the Geode Native project.

So, I am putting a list of pros and cons:

Pros:

  *   Several new features added:
 *   C++14 features: 
https://en.cppreference.com/w/cpp/14#New_language_features
 *   C++17 features: 
https://en.cppreference.com/w/cpp/17#New_language_features
  *   Some of the interesting features are:
 *   Function return type deduction.
 *   Improved constexpr functions.
 *   Variable templates.
 *   Generic lambdas.
 *   Lambda capture expressions.
 *   [[deprecated]]
 *   Shared mutexes/locks.
 *   std::make_unique
 *   Nested namespace definitions.
 *   Structured bindings.
 *   variant.
 *   any.
 *   optional.

Cons:
  *   Some users might have older compilers which does not implement all C++ 17 
features.

Thanks,
Mario.


From: Mario Salazar de Torres 
Sent: Tuesday, May 4, 2021 7:34 PM
To: dev@geode.apache.org 
Subject: DISCUSSION: Geode Native C++ 17 adoption

Hi everyone,

Currently Geode Native uses C++11 standard. It has been quite some time since 
the standard was released and as of today the latest standard is C++20.
As part of another discussion, some users in the community were wondering if 
it's the time to switch to C++17 in the Geode Native project.

So, I am putting a list of pros and cons:

Pros:

  *   Several new features added:
 *   C++14 features: 
https://en.cppreference.com/w/cpp/14#New_language_features
 *   C++17 features: 
https://en.cppreference.com/w/cpp/17#New_language_features
  *   Some of the interesting features are:
 *   Function return type deduction.
 *   Improved constexpr functions.
 *   Variable templates.
 - Generic lambdas.
 - Lambda capture expressions.
 - [[deprecated]]
 - Shared mutexes/locks.
 - std::make_unique
C++17 - 
cppreference.com
This page was last modified on 20 October 2020, at 04:39. This page has been 
accessed 106,431 times. Privacy policy; About cppreference.com; Disclaimers
en.cppreference.com



C++14 - 
cppreference.com
New language features . variable templates; generic lambdas; lambda 
init-capture new/delete elision relaxed restrictions on constexpr functions; 
binary literals
en.cppreference.com


Re: DISCUSSION: Geode Native C++ 17 adoption

[Charlie Black]

Another viewpoint is this is a library and that library can be used by older 
applications.   So being cutting edge on the complier on the library does not 
increase the adoption of Geode.

Agree with Alberto that users mailing lists should be consulted.

Charlie


From: Alberto Gomez 
Date: Tuesday, May 4, 2021 at 11:29 AM
To: dev@geode.apache.org 
Subject: Re: DISCUSSION: Geode Native C++ 17 adoption
Hi,

Here come my two cents.

To me, upgrading to C++17 is a no brainer given that C++11 is quite old and 
C++17 has lots of new features, performance improvements and bug fixes.

The only thing that could prevent us from doing so is having lots of users that 
are running the native client in a platform that does not have a C++17 
compiler. Which leads me to the question: should we move this discussion to the 
user mailing list?

Alberto

From: Mario Salazar de Torres 
Sent: Tuesday, May 4, 2021 7:45 PM
To: dev@geode.apache.org 
Subject: Re: DISCUSSION: Geode Native C++ 17 adoption

Hi everyone,

Sorry for the previous email, I did send it before finishing it by mistake.

Currently Geode Native uses C++11 standard. It has been quite some time since 
the standard was released and as of today the latest standard is C++20.
As part of another discussion, some users in the community were wondering if 
it's the time to switch to C++17 in the Geode Native project.

So, I am putting a list of pros and cons:

Pros:

  *   Several new features added:
 *   C++14 features: 
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fen.cppreference.com%2Fw%2Fcpp%2F14%23New_language_features&data=04%7C01%7Ccblack%40vmware.com%7C99bf8329d4ed48682b2b08d90f2a8d9f%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637557497738349382%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=rs4Pc4zW6bo2oyoaAzPDLA4gUAzm2Q8%2FRNDgLw3YxsY%3D&reserved=0
 *   C++17 features: 
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fen.cppreference.com%2Fw%2Fcpp%2F17%23New_language_features&data=04%7C01%7Ccblack%40vmware.com%7C99bf8329d4ed48682b2b08d90f2a8d9f%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637557497738349382%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=dFzhj5YgERe3wtVAAbH1BukTv7Q5bcNVJNWgfPQfMlY%3D&reserved=0
  *   Some of the interesting features are:
 *   Function return type deduction.
 *   Improved constexpr functions.
 *   Variable templates.
 *   Generic lambdas.
 *   Lambda capture expressions.
 *   [[deprecated]]
 *   Shared mutexes/locks.
 *   std::make_unique
 *   Nested namespace definitions.
 *   Structured bindings.
 *   variant.
 *   any.
 *   optional.

Cons:
  *   Some users might have older compilers which does not implement all C++ 17 
features.

Thanks,
Mario.


From: Mario Salazar de Torres 
Sent: Tuesday, May 4, 2021 7:34 PM
To: dev@geode.apache.org 
Subject: DISCUSSION: Geode Native C++ 17 adoption

Hi everyone,

Currently Geode Native uses C++11 standard. It has been quite some time since 
the standard was released and as of today the latest standard is C++20.
As part of another discussion, some users in the community were wondering if 
it's the time to switch to C++17 in the Geode Native project.

So, I am putting a list of pros and cons:

Pros:

  *   Several new features added:
 *   C++14 features: 
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fen.cppreference.com%2Fw%2Fcpp%2F14%23New_language_features&data=04%7C01%7Ccblack%40vmware.com%7C99bf8329d4ed48682b2b08d90f2a8d9f%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637557497738359379%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=yeadKm2sq%2BZI3e382PQ3NlpLxyjfH0vCO7WEQyT2W%2FM%3D&reserved=0
 *   C++17 features: 
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fen.cppreference.com%2Fw%2Fcpp%2F17%23New_language_features&data=04%7C01%7Ccblack%40vmware.com%7C99bf8329d4ed48682b2b08d90f2a8d9f%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637557497738359379%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=MaJ77vnuas3whxAzYCMxfFo5b2I5QJzNkSmT5iL%2FTmI%3D&reserved=0
  *   Some of the interesting features are:
 *   Function return type deduction.
 *   Improved constexpr functions.
 *   Variable templates.
 - Generic lambdas.
 - Lambda capture expressions.
 - [[deprecated]]
 - Shared mutexes/locks.
 - std::make_unique
C++17 - 
cppreference.com

Re: DISCUSSION: Geode Native C++ 17 adoption

[Mario Salazar de Torres]

Hi,

I am forwarding this discussion to the user list, as I also agree, they should 
also give their view on this.

Additionally, I've been doing some tests and one downside is that the following 
classes/functions have been removed in C++17:

  *   std::unary_function and binary_function have been removed.
There are some classes which for some reason I don't know inherit from both 
unary_function and binary_function.
This is not really needed, so the solution would be as easy as not using them 
anymore.
  *   std::random_shuffle has been removed and replaced by std::shuffle.
  *   std::ptr_fun has been removed but can be simply replaced by a lambda.
  *   URNG template has changed. Some minimal effort should be invested in 
changing this.

Good news is that these changes does not affect to any part of the public API, 
and changes required due to above-described changes are minimal.

Thanks,
Mario.

From: Charlie Black 
Sent: Tuesday, May 4, 2021 9:33 PM
To: dev@geode.apache.org 
Subject: Re: DISCUSSION: Geode Native C++ 17 adoption

Another viewpoint is this is a library and that library can be used by older 
applications.   So being cutting edge on the complier on the library does not 
increase the adoption of Geode.

Agree with Alberto that users mailing lists should be consulted.

Charlie


From: Alberto Gomez 
Date: Tuesday, May 4, 2021 at 11:29 AM
To: dev@geode.apache.org 
Subject: Re: DISCUSSION: Geode Native C++ 17 adoption
Hi,

Here come my two cents.

To me, upgrading to C++17 is a no brainer given that C++11 is quite old and 
C++17 has lots of new features, performance improvements and bug fixes.

The only thing that could prevent us from doing so is having lots of users that 
are running the native client in a platform that does not have a C++17 
compiler. Which leads me to the question: should we move this discussion to the 
user mailing list?

Alberto

From: Mario Salazar de Torres 
Sent: Tuesday, May 4, 2021 7:45 PM
To: dev@geode.apache.org 
Subject: Re: DISCUSSION: Geode Native C++ 17 adoption

Hi everyone,

Sorry for the previous email, I did send it before finishing it by mistake.

Currently Geode Native uses C++11 standard. It has been quite some time since 
the standard was released and as of today the latest standard is C++20.
As part of another discussion, some users in the community were wondering if 
it's the time to switch to C++17 in the Geode Native project.

So, I am putting a list of pros and cons:

Pros:

  *   Several new features added:
 *   C++14 features: 
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fen.cppreference.com%2Fw%2Fcpp%2F14%23New_language_features&data=04%7C01%7Ccblack%40vmware.com%7C99bf8329d4ed48682b2b08d90f2a8d9f%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637557497738349382%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=rs4Pc4zW6bo2oyoaAzPDLA4gUAzm2Q8%2FRNDgLw3YxsY%3D&reserved=0
 *   C++17 features: 
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fen.cppreference.com%2Fw%2Fcpp%2F17%23New_language_features&data=04%7C01%7Ccblack%40vmware.com%7C99bf8329d4ed48682b2b08d90f2a8d9f%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637557497738349382%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=dFzhj5YgERe3wtVAAbH1BukTv7Q5bcNVJNWgfPQfMlY%3D&reserved=0
  *   Some of the interesting features are:
 *   Function return type deduction.
 *   Improved constexpr functions.
 *   Variable templates.
 *   Generic lambdas.
 *   Lambda capture expressions.
 *   [[deprecated]]
 *   Shared mutexes/locks.
 *   std::make_unique
 *   Nested namespace definitions.
 *   Structured bindings.
 *   variant.
 *   any.
 *   optional.

Cons:
  *   Some users might have older compilers which does not implement all C++ 17 
features.

Thanks,
Mario.


From: Mario Salazar de Torres 
Sent: Tuesday, May 4, 2021 7:34 PM
To: dev@geode.apache.org 
Subject: DISCUSSION: Geode Native C++ 17 adoption

Hi everyone,

Currently Geode Native uses C++11 standard. It has been quite some time since 
the standard was released and as of today the latest standard is C++20.
As part of another discussion, some users in the community were wondering if 
it's the time to switch to C++17 in the Geode Native project.

So, I am putting a list of pros and cons:

Pros:

  *   Several new features added:
 *   C++14 features: 
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fen.cppreference.com%2Fw%2Fcpp%2F14%23New_language_features&data=04%7C01%7Ccblack%40vmware.com%7C99bf8329d4ed48682b2b08d90f2a8d9f%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637557497738359379%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=yeadKm2sq%2BZI3e382PQ3NlpLxyjfH0vCO7WEQ

Re: Region data corruption due to missing PdxTypes

[Anthony Baker]

Retaining local pox types in the client after a disconnect will cause problems 
as you observed.  Take a look at the “ON_DISCONNECT_CLEAR_PDXTYPEIDS” property 
to improve this.

Anthony


> On May 4, 2021, at 4:36 AM, Mario Salazar de Torres 
>  wrote:
> 
> Hi everyone,
> 
> While debugging some coredumps in the native client related to 
> PdxTypeRegistry cleanup, I tried to reproduce the scenario with the Java 
> client API to see how it was handled.
> Thing is I've noticed that this scenario in the Java client might lead to 
> Geode storing a corrupted entry, meaning that queries won't work on those 
> regions containing corrupted entries.
> And with corrupted entries, I refer to entries using a missing PdxType. The 
> scenario involves a cluster restart. It's described below:
> 
>  1.  Start a cluster with 1 locator and 3 servers, and persistence is 
> disabled for PdxTypes.
>  2.  Setup a region called "test-region" with persistence disabled. It 
> doesn't mind whether is replicated or partitioned.
>  3.  In the client, instantiate the client region with PROXY region shortcut 
> and establish the connection toward the cluster.
>  4.  In the client, create a PdxInstance and put in into the "test-region" 
> with key "test".
>  5.  In the client, get the entry which key is "test", which turns out to be 
> the PdxInstance inserted in step 4.
>  6.  At this point, cluster is restarted, meaning that all the data is lost, 
> included PdxTypes.
>  7.  In the client, the PdxInstance obtained in step 5 is put into 
> "test-region" with key "test2"
>  8.  In the client, the following query is executed: "SELECT * FROM 
> /test-region WHERE value = -1".
> Such query fails with the message "Unknown pdx type=" and it 
> won't work until the corrupted entry is removed.
> 
> Also, the above scenario could be solved by enabling persistence for 
> PdxTypes, but if you have an unrecoverable issue in your cluster and you need 
> to spin up a backup,
> it could happen that PdxInstance's PdxType obtained step 5 is not present in 
> the backup, leading to the entry being inserted but, yet again, the PdxType 
> being missing.
> 
> It's worth mentioning that in the native client, this scenario currently 
> results in a coredump, but no data corruption,
> given that after losing the connection towards the cluster PdxTypeRegistry is 
> cleaned up and PdxTypes are obtained with its ID, rather than directly using 
> the object.
> 
> My question here are:
> 
>  *   Have you seen this issue before?
>  *   Is there a way to verify that PdxTypes are present in the cluster before 
> writing an entry which holds some PdxInstances?
> 
> Thanks,
> Mario.

Re: DISCUSSION: Geode Native C++ 17 adoption

[Jacob Barrett]

On May 4, 2021, at 12:33 PM, Charlie Black 
mailto:cbl...@vmware.com>> wrote:

Another viewpoint is this is a library and that library can be used by older 
applications.   So being cutting edge on the complier on the library does not 
increase the adoption of Geode.

The public API can’t change in a minor release in a way that would break, so 
all previous C++11 based APIs would remain intact. Older applications utilizing 
C++11 would continue to work. We can define any new public C++17 APIs in way 
that utilizes the feature macros from C++11 and newer to enable features at 
runtime depending on the compiler support.

The real problem is the runtime libraries for C++17 support need updating. The 
older application needs to be compiled with the newer compiler and runtime 
library. This mostly affects older OSs, like RHEL 7 and Ubuntu 18.04. On RHEL 7 
we are already require using devtoolkit 4 to get the GCC 5 compiler and C++11 
runtimes. To support this bump to C++17 RHEL 7 needs to use Devtoolkit 9. 
Similar is true with Ubuntu 18.04, which provides the ability to install GCC 9, 
though it isn’t default. Shockingly this isn’t really a problem for Windows.

-Jake