Re: PROPOSAL: Remove WAN TX Batching Serialization Changes

[Alberto Gomez]

Hi,

Jake, why do you say the feature is not complete in 1.14.0? In my view, it 
works as it was designed and as documented.

I do not think it makes sense to remove a feature shipped in the 1.14.0 release 
that customers might already be using.

If the urgent issue to solve is the unnecessary serialization overhead when the 
WAN or AEQ events are part of a transaction and the sender has not enabled TX 
batching, I propose we try to fix just this in 1.14.1.

Alberto


From: Jacob Barrett 
Sent: Tuesday, September 21, 2021 11:13 PM
To: dev@geode.apache.org 
Subject: PROPOSAL: Remove WAN TX Batching Serialization Changes

Devs,

In addition to my discussion regarding the modularization of the WAN TX 
batching implementation I would like to propose that we remove the 
serialization changes that went into 1.14 to support it. Since the feature is 
not complete in 1.14 this should only impact the associated tests in 1.14. I 
want to do this to eliminate the necessary serialization of the of the 
transaction ID and last event flags as well as the boolean to determine if 
there is a transaction ID. As implemented right now this data is serialized for 
both WAN and AEQ sender events that are part of a transaction regardless of the 
enablement of TX batching on the sender. The transaction ID contains both the 4 
byte counter and large membership ID.
https://github.com/apache/geode/blob/develop/geode-core/src/main/java/org/apache/geode/internal/cache/wan/GatewaySenderEventImpl.java#L712

Since this went out in 1.14.0 the removal would be treated like any other 
upgrade to the protocol and a 1.14.1 version would not read or write any of 
those bites. When talking to exactly a 1.14.0 version the implementation would 
write only the false flag and read the flag and ignore the rest as necessary. 
The tests related to TX batching would also need to be disabled.

Something like this:

  public void toData(DataOutput out,
  SerializationContext context) throws IOException {
// intentionally skip 1.14.0
toDataPre_GEODE_1_14_0_0(out, context);
  }

  public void toDataPre_GEODE_1_14_1_0(DataOutput out,
  SerializationContext context) throws IOException {
toDataPre_GEODE_1_14_0_0(out, context);
DataSerializer.writeBoolean(false);
  }

  public void fromData(DataInput in, DeserializationContext context)
  throws IOException, ClassNotFoundException {
fromDataPre_GEODE_1_14_1_0(in, context);
  }

  public void fromDataPre_GEODE_1_14_1_0(DataInput in, DeserializationContext 
context)
  throws IOException, ClassNotFoundException {
fromDataPre_GEODE_1_14_0_0(in, context);
if (version == KnownVersion.GEODE_1_14_0.ordinal()) {
  if (hasTransaction) {
DataSerializer.readBoolean(DataSerializer.readBoolean(in));
context.getDeserializer().readObject(in);
  }
}
  }

I would also propose that if 1.15.0 looks like it will ship without the 
modularization changes that we at least address the serialization changes here 
in a way that does not affect all gateways, WAN or AEQ.

If accepted I will write up two JIRAs, one to address the 1.14 removal and the 
other as a blocker on 1.15 to address the serialization issues.

Ok, chime in!

-Jake

Re: PROPOSAL: Remove WAN TX Batching Serialization Changes

[Jacob Barrett]

> On Sep 22, 2021, at 12:31 AM, Alberto Gomez  wrote:
> 
> Hi,
> 
> Jake, why do you say the feature is not complete in 1.14.0? In my view, it 
> works as it was designed and as documented.

Sorry, I was misinformed and misjudge the state. I meant no disrespect.

> I do not think it makes sense to remove a feature shipped in the 1.14.0 
> release that customers might already be using.

I agree!

> If the urgent issue to solve is the unnecessary serialization overhead when 
> the WAN or AEQ events are part of a transaction and the sender has not 
> enabled TX batching, I propose we try to fix just this in 1.14.1.

Yes, let me look at an alternative approach for 1.14 that can reduce that 
overhead when the sender is not batching transactions.

I think it makes send to scratch the original proposal. I will just write up a 
JIRA and PR for the protocol changes agains develop and then we can move them 
back to 1.14.1.

-Jake

Re: PROPOSAL: Remove WAN TX Batching Serialization Changes

[Alberto Gomez]

No worries, Jake.

The steps you are proposing sound good to me. Let me know if I can be of any 
help.

Alberto

From: Jacob Barrett 
Sent: Wednesday, September 22, 2021 7:44 PM
To: dev@geode.apache.org 
Subject: Re: PROPOSAL: Remove WAN TX Batching Serialization Changes



> On Sep 22, 2021, at 12:31 AM, Alberto Gomez  wrote:
>
> Hi,
>
> Jake, why do you say the feature is not complete in 1.14.0? In my view, it 
> works as it was designed and as documented.

Sorry, I was misinformed and misjudge the state. I meant no disrespect.

> I do not think it makes sense to remove a feature shipped in the 1.14.0 
> release that customers might already be using.

I agree!

> If the urgent issue to solve is the unnecessary serialization overhead when 
> the WAN or AEQ events are part of a transaction and the sender has not 
> enabled TX batching, I propose we try to fix just this in 1.14.1.

Yes, let me look at an alternative approach for 1.14 that can reduce that 
overhead when the sender is not batching transactions.

I think it makes send to scratch the original proposal. I will just write up a 
JIRA and PR for the protocol changes agains develop and then we can move them 
back to 1.14.1.

-Jake

Re: PROPOSAL: Remove WAN TX Batching Serialization Changes

[Jacob Barrett]

For closure here is the PR:
https://github.com/apache/geode/pull/6892


On Sep 22, 2021, at 10:48 AM, Alberto Gomez 
mailto:alberto.go...@est.tech>> wrote:

No worries, Jake.

The steps you are proposing sound good to me. Let me know if I can be of any 
help.

Alberto

From: Jacob Barrett mailto:jabarr...@vmware.com>>
Sent: Wednesday, September 22, 2021 7:44 PM
To: dev@geode.apache.org 
mailto:dev@geode.apache.org>>
Subject: Re: PROPOSAL: Remove WAN TX Batching Serialization Changes



On Sep 22, 2021, at 12:31 AM, Alberto Gomez 
mailto:alberto.go...@est.tech>> wrote:

Hi,

Jake, why do you say the feature is not complete in 1.14.0? In my view, it 
works as it was designed and as documented.

Sorry, I was misinformed and misjudge the state. I meant no disrespect.

I do not think it makes sense to remove a feature shipped in the 1.14.0 release 
that customers might already be using.

I agree!

If the urgent issue to solve is the unnecessary serialization overhead when the 
WAN or AEQ events are part of a transaction and the sender has not enabled TX 
batching, I propose we try to fix just this in 1.14.1.

Yes, let me look at an alternative approach for 1.14 that can reduce that 
overhead when the sender is not batching transactions.

I think it makes send to scratch the original proposal. I will just write up a 
JIRA and PR for the protocol changes agains develop and then we can move them 
back to 1.14.1.

-Jake