This is set in the constructor and used in `toData()` and `fromData()` but
other than that it's never accessed or modified, so it can probably be removed.
[ Full content available at: https://github.com/apache/geode/pull/4689 ]
This message was relayed via gitbox.apache.org for
This is currently never used, so unless it's going to be needed for the API
story, it should probably be removed.
[ Full content available at: https://github.com/apache/geode/pull/4689 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
It might be worth including tests to confirm that `initMessage()` correctly
sets the `processorId` and doesn't call `enableSevereAlertProcessing()` on the
`ReplyProcessor` if the processor is null, a test to confirm that `send()`
throws an exception if `putOutgoing(this)` returns a non-null
`this.isSevereAlertCompatible()` is always true for this class, so it can
probably be removed from this if statement.
[ Full content available at: https://github.com/apache/geode/pull/4689 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
Remove comment.
[ Full content available at: https://github.com/apache/geode/pull/4689 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
After talking with Gester - we should remove notify only, transactional related
stuff, and notifyGatewaySender logic, make sure that doLocalClear() throws
ForceReattemptException instead of PartitionedRegionException, and use a while
loop to get the lock/check primary (see lucine
Thank you for submitting a contribution to Apache Geode.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the
commit message?
- [ ] Has
After talking with Gester - we should remove notify only and the transactional
related stuff, make sure that doLocalClear() throws ForceReattemptException
instead of PartitionedRegionException, and use a while loop to get the
lock/check primary (see lucine IndexRepositoryFactory.java)
[ Full
Also, is there a way we can convert to awaitility so there is no chance a test
can hang forever?
[ Full content available at: https://github.com/apache/geode/pull/4690 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
Only one `PubSub` is created for the lifetime of the adapter. Each
`ExecutionContextHandler` receives a reference to it.
[ Full content available at: https://github.com/apache/geode/pull/4682 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
Ditto above.
[ Full content available at: https://github.com/apache/geode/pull/4682 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
Same ditto
[ Full content available at: https://github.com/apache/geode/pull/4682 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
It's a good point. We realize that `context` is very overloaded right now and
we're planning on refactoring in the future.
[ Full content available at: https://github.com/apache/geode/pull/4682 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
We're coding to the interface and not the concrete class. It could easily be
changed if the need arises.
[ Full content available at: https://github.com/apache/geode/pull/4682 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
I wonder if one should expose the `PubSub` world here... maybe have the context
have a `publish` method, which then delegates to the internal `PubSub`. To me
it is a little like feature envy... Where instead of having the
`ExecutionHandlerContext` handle the publish, we first have to know that
there's some dunit tests fail from time to time on different test. I think it's
unstable in base branch i.e feature/GEODE-7665
[ Full content available at: https://github.com/apache/geode/pull/4638 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
Can we then just add an interface and an implementation... It is easier to do
this BEFORE rather than after. Also it makes it easier to achieve loose
coupling.
[ Full content available at: https://github.com/apache/geode/pull/4682 ]
This message was relayed via gitbox.apache.org for
I don't think that will work since `Comparable` only uses a single type and
`isEqualTo` requires multiple types.
[ Full content available at: https://github.com/apache/geode/pull/4682 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
Would we not prefer a checked exception here?
[ Full content available at: https://github.com/apache/geode/pull/4682 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
Not really, but since this pattern is used throughout all the commands I think
it would be better to make that change in a separate PR and cover all the
commands.
[ Full content available at: https://github.com/apache/geode/pull/4682 ]
This message was relayed via gitbox.apache.org for
Could we please raise one and address accordingly
[ Full content available at: https://github.com/apache/geode/pull/4682 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
Keep the last known 'DurableClienAttributes' cached within the
'InternalDistributedMember' class to avoid the overhead of creating a
new instance every time.
Thank you for submitting a contribution to Apache Geode.
In order to streamline the review of the contribution we ask you
to ensure the
It ensures that the `PubSub` instance is fully constructed before use - since
the Function requires a reference to it.
[ Full content available at: https://github.com/apache/geode/pull/4682 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
I think that's just a different approach - also valid, but just different
[ Full content available at: https://github.com/apache/geode/pull/4682 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
You are correct.
That said.. What is the max number of Pub/Sub combinations we can expect? How
does this approach scale?
Maybe two maps > and > ?
[ Full content available at: https://github.com/apache/geode/pull/4682 ]
This message was relayed via gitbox.apache.org for
Fixed code
[ Full content available at: https://github.com/apache/geode/pull/4682 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
I'm not sure I understand. We need to know two things:
1 - has a client already subscribed to a channel
2 - how many subscriptions a client currently has
In order to address both of these concerns, the `findSubscribers` method would
probably need to be inlined which seems like it would be less
[ pull request closed by gesterzhou ]
[ Full content available at: https://github.com/apache/geode/pull/4638 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
[ pull request closed by igodwin ]
[ Full content available at: https://github.com/apache/geode/pull/4686 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
That is true.
[ Full content available at: https://github.com/apache/geode/pull/4682 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
Let me rephrase, given that there are no dependencies as input parameters, the
registering of Functions can happen at construction time.
[ Full content available at: https://github.com/apache/geode/pull/4682 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
So, you can 100% guarantee that the fields are ALWAYS non-null?
[ Full content available at: https://github.com/apache/geode/pull/4682 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
another approach is that the equals method is `this.channel.equals(channel`.
This way the constructor can ensure that the instance channel field is non-null
AND your `isEqualsTo` is null-safe
[ Full content available at: https://github.com/apache/geode/pull/4682 ]
This message was relayed via
Could we leave optimization until it's actually determined that it is needed?
[ Full content available at: https://github.com/apache/geode/pull/4682 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
I agree, premature optimization But I'd be interested in what the "normal"
usage on this would be. Maybe we add a load testing ticket on this, to see what
load this construct can take. Is it 100's or 1000's or even 10,000's...
But I think it would be good to understand when it will slow
Thank you for submitting a contribution to Apache Geode.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the
commit message?
- [ ] Has
Thank you for submitting a contribution to Apache Geode.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the
commit message?
- [ ] Has
[ pull request closed by rhoughton-pivot ]
[ Full content available at: https://github.com/apache/geode/pull/4685 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
Also, you can use `new PubSub` as the `registerFunction` can (and should) be
called in the constructor of `PubSub` otherwise one is exposing knowledge that
does not need to be exposed.
[ Full content available at: https://github.com/apache/geode/pull/4682 ]
This message was relayed via
Thank you for submitting a contribution to Apache Geode.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the
commit message?
- [ ] Has
But I don't see any immediate value in changing it.
[ Full content available at: https://github.com/apache/geode/pull/4682 ]
This message was relayed via gitbox.apache.org for
notifications@geode.apache.org
41 matches
Mail list logo
