make subscription return statement event instead of bytes#11139
make subscription return statement event instead of bytes#11139
Conversation
Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
| /// The number of statements present in the store matching the subscription filter at the time | ||
| /// of subscription. This is sent immediately upon subscription, before any matching statements | ||
| /// are sent. | ||
| NumMatchingStatements(u32), |
There was a problem hiding this comment.
My suggestion here is to use a struct with more metadata (statements, how many found, how many left to send, etc). If there is nothing, we can send the struct with zeros on first connection
There was a problem hiding this comment.
Agree here and we have already discussed with Alex that it is possible just start sending statements right away and pass empty list if there is no matching statement. However WDYT if we include also number of remaining statements that will be followed in subsequent batches. So instead of Vec<Bytes> we could return StatementsBatch struct:
StatementsBatch {
statements: Vec<Bytes>,
remaining: Int
}
StatementBatch.empty = StatementsBatch { [], 0 }
-
If remaining is greater than zero then client will expect next batch to come in reasonable timeout interval.
-
If for some reason statements for the next batch disappeared from the store but node sent remaining > 0 in previous batch then it MUST send StatementBatch.empty.
-
Clients always expect first batch once subscription established. Node MUST sent StatementBatch.empty if there is no matching statements.
I think that logic besides just monitoring statements allows also syncing mechanism: when a client just needs a reliable way to get some old statements before deciding what to do next.
There was a problem hiding this comment.
Thinking about this, I don't think remaining helps much because it can becomestale inaccurate, as you receive more statements on nodes.
The users of the API need to handle the fact that statements can arrive at any time after the subscription was initiated, there is no-point in you knowing "remaining" because new statements can arrive on node that invalidate "remaining" at any time.
There was a problem hiding this comment.
remaning shouldn't be accurate. The purpose is to optimise syncing cases for clients and give a hint is there follow up statements or not.
There was a problem hiding this comment.
@ERussel: Implemented your suggestion, the PR description is now updated with this as well.
AndreiEres
left a comment
There was a problem hiding this comment.
I very like the idea of giving an immediate response if there are no statements. But I don't like that if there are statements we still spend one extra message to say that they exist instead of sending them. Can be tragical on low internet connection.
Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
Makes sense, modified so that we just send and empty array at the begining if there are no statements. |
Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
## Problem
In the current implementation of the subscription, applications have no
way to tell if there is anything in the store for their subscription,
because we are not sending any information when zero statements match
the filter, so they have the problem of not knowing if the subscription
will produce any items or if it is just slow.
Another worthy optimisation is that, when we connect we can send more
than one statement in one notification rather than send them one by one,
which creates more churn and is slower.
## Proposal.
Modify the API, so instead of returning a stream of bytes that represent
scale encoded statements, to return
```
pub enum StatementEvent {
/// A batch of statements matching the subscription filter. Each entry is a SCALE-encoded
/// statement.
NewStatements(Vec<Bytes>),
}
```
When subscription is initiated if there are no matching statements in
the store we send an empty array..
This changes slightly the json-rpc schema of the message:
## Before this change:
- Receiving a statement
```
{
"jsonrpc": "2.0",
"method": "statement_statement",
"params": {
"subscription": 2759293729543571,
"result": "0x1000010000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000201000000ffffffff040000000000000000000000000000000000000000000000000000000000000000050101010101010101010101010101010101010101010101010101010101010101"
}
}
```
## After this change:
- When there are no matching statements in the store you first receive
an empty array and as new matching statements arrive in the node they
get forwarded to the client.
```
{
"jsonrpc": "2.0",
"method": "statement_statement",
"params": {
"subscription": 4851578855668545,
"result": {
"event": "newStatements",
"data": {
"statements": [],
"remaining": 0
}
}
}
}
```
- If there are matching statements in the store you receive them in
batches of newStatements events, with remaining telling you how many
statements you have remaining, this guarantees you that the subscription
will receive at least this amount of statements.
```
{
"jsonrpc": "2.0",
"method": "statement_statement",
"params": {
"subscription": 1710164133533157,
"result": {
"event": "newStatements",
"data": {
"statements": [
"0x1000010000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000201000000ffffffff040000000000000000000000000000000000000000000000000000000000000000050202020202020202020202020202020202020202020202020202020202020202",
"0x1000010000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000201000000ffffffff040000000000000000000000000000000000000000000000000000000000000000050101010101010101010101010101010101010101010101010101010101010101"
],
"remaining": 10
}
}
}
}
```
- If new statements arrive in the store they get delivered as they are
without any `remaining` information.
```
{
"jsonrpc": "2.0",
"method": "statement_statement",
"params": {
"subscription": 2661920166788434,
"result": {
"event": "newStatements",
"data": {
"statements": [ "0x1000010000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000201000000ffffffff040000000000000000000000000000000000000000000000000000000000000000050101010101010101010101010101010101010101010101010101010101010101"
]
}
}
}
}
```
---------
Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io>
(cherry picked from commit 30d11a7)
|
Successfully created backport PR for |
Backport #11139 into `stable2603` from alexggh. See the [documentation](https://github.com/paritytech/polkadot-sdk/blob/master/docs/BACKPORT.md) on how to use this bot. <!-- # To be used by other automation, do not modify: original-pr-number: #${pull_number} --> Signed-off-by: Alexandru Gheorghe <alexandru.gheorghe@parity.io> Co-authored-by: Alexandru Gheorghe <49718502+alexggh@users.noreply.github.com>
Problem
In the current implementation of the subscription, applications have no way to tell if there is anything in the store for their subscription, because we are not sending any information when zero statements match the filter, so they have the problem of not knowing if the subscription will produce any items or if it is just slow.
Another worthy optimisation is that, when we connect we can send more than one statement in one notification rather than send them one by one, which creates more churn and is slower.
Proposal.
Modify the API, so instead of returning a stream of bytes that represent scale encoded statements, to return
When subscription is initiated if there are no matching statements in the store we send an empty array..
This changes slightly the json-rpc schema of the message:
Before this change:
After this change:
remaininginformation.