This repository has been archived by the owner on May 10, 2022. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
feat(security): handle list mechanism response #136
feat(security): handle list mechanism response #136
Changes from 23 commits
1a0028f
5a48762
8248624
4d8073d
a72a0af
c16e8bb
ce86a28
f787f91
ebb2c54
a7af9f3
07a0361
dfe64f6
138757c
ce80e5d
5bbe906
7b7b10d
deeda00
3781c1b
082d626
7e1c58d
a967161
4fc5f01
1c3aa16
fcc0ea0
1d956f8
c928bd5
28b6c47
4a2a4b9
d9a1c46
84bf4f4
a6d0252
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Negotiation should be set dynamically by the user timeout. It's under the user's control that how long an RPC should last.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it must under the user's control. The negotiation process should be not visable to users
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your design here binds the responsibility of negotiation to ReplicaSession again. Why couldn't ReplicaSessionInterceptor hold the pending queue?
negotiationPendingSend is declared final, but you offer items here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's hard to hold pending queue by
ReplicaSessionInterceptor
.ReplicaSessionInterceptor
is a singleton, which will serves all of the ReplicaSession. So if we do this like you said, we must use a coarse-grained lock, which will produce low effiencyReplicaSessionInterceptor
to do this, we must implement a lot of logic in it. For example timeout handling, which will mantain a timeout pendingResponse queue and a PendingSend, and so on. It's too complicated, and there is a lot of overlap code with ReplicaSession. And we should modify ReplicaSession to support it.