geode
geode copied to clipboard
Wip/wan tx grouping module 1
For all changes:
-
[ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?
-
[ ] Has your PR been rebased against the latest commit within the target branch (typically
develop
)? -
[ ] Is your initial contribution a single, squashed commit?
-
[ ] Does
gradlew build
run cleanly? -
[ ] Have you written or updated unit tests to verify your changes?
-
[ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
@albertogpz Can you please rebase this from develop. There as a fixed added there to address issues with Gradle builds.
@albertogpz we need to extract out the TX batching specific elements form the GatewaySenderEventImpl
object. I replaced it with a implementation defined metadata object. See TxBatchMetadata
from my previous example on making this a module. This gets set on the event object when constructed via an override on AbstractGatewaySenderEventProcessor
. See example here.
Inlined a few comments. Overall looks like a good direction! Here are some general comments and tips:
1. Prefer interfaces over abstract methods. 2. Standardize on one library instead of mixing them (such as AssertJ for assertions). 3. Is there a better word than "txgrouping" for module name and packages? 4. Declare variables and APIs with interfaces rather than implementations. 5. Always look for an interface to use instead of an implementation (such as InternalCache instead of GemFireCacheImpl) in both tests and product code. 6. Always minimize the scope of anything (class, method, variable, constant).
Thanks for your comments. Regarding the name, I do not have one that I really like. Other alternatives could be: txeventsgrouping, grouptxevents, txaware,... If you like any of these or have any other proposal, please, let me know.
@albertogpz we need to extract out the TX batching specific elements form the
GatewaySenderEventImpl
object. I replaced it with a implementation defined metadata object. SeeTxBatchMetadata
from my previous example on making this a module. This gets set on the event object when constructed via an override onAbstractGatewaySenderEventProcessor
. See example here.
@pivotal-jbarrett As I said in a previous e-mail, I do not think we can make these changes without breaking backward compatibility. The feature was production ready at 1.14 so we can expect that it is used by customers. Therefore, we cannot just ignore those changes when interworking with 1.14 servers. Am I missing something?
this PR appears to be abandoned, can it be closed?
this PR appears to be abandoned, can it be closed?
It is not abandoned. Still waiting for comments after review changes. I have rebased the branch to remove the conflicts with develop.