Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stream Mirrors, Sources and Updates Support #1932

Merged
merged 12 commits into from
Feb 24, 2021
Merged

Stream Mirrors, Sources and Updates Support #1932

merged 12 commits into from
Feb 24, 2021

Conversation

derekcollison
Copy link
Member

Also various fixes, stability improvements, etc.

/cc @nats-io/core

Signed-off-by: Derek Collison <derek@nats.io>
Signed-off-by: Derek Collison <derek@nats.io>
Signed-off-by: Derek Collison <derek@nats.io>
Signed-off-by: Derek Collison <derek@nats.io>
Signed-off-by: Derek Collison <derek@nats.io>
Signed-off-by: Derek Collison <derek@nats.io>
Add in proper support for stream updates in clustered mode.
Don't send API updates without subjects, caused GW parser errors.
Stream internal loops use their own clients now.

Signed-off-by: Derek Collison <derek@nats.io>
// Headers for published messages.
const (
JSMsgId = "Nats-Msg-Id"
JSExpectedStream = "Nats-Expected-Stream"
JSExpectedLastSeq = "Nats-Expected-Last-Sequence"
JSExpectedLastMsgId = "Nats-Expected-Last-Msg-Id"
JSStreamSource = "Nats-Stream-Source"
Copy link
Contributor

@ripienaar ripienaar Feb 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw these and wasnt sure what the user story this fixes? Would the user parse this subjet (its the ack subject) to figure out metadata? This would have original time stamp I guess, the sourced timestamp would be different?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They could, we need it for sources to properly figure out where to pick back up for any given source. We walk backwards through our own stream and look at that header for stream sequence for the source stream. Then we recreate the consumer with that starting sequence +1.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, its like 70 bytes overhead per message. We could pick out just the bits we need and put it in a header we dont show the user - and so with a much shorter name?

Usually I am not too concerned about messages sizes - a lot less than you :P - but 70/message is a lot.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

| Stream         | Storage | Template | Consumers | Messages | Bytes   | Lost | Deleted | Cluster              |
+----------------+---------+----------+-----------+----------+---------+------+---------+----------------------+
| ORDERS_6       | File    |          | 5         | 0        | 0 B     | 0    | 0       | n1-c1, n2-c1, n3-c1* |
| ORDERS_7       | File    |          | 5         | 0        | 0 B     | 0    | 0       | n1-c1, n2-c1*, n3-c1 |
| ORDERS_2       | File    |          | 5         | 0        | 0 B     | 0    | 0       | n1-c1, n2-c1*, n3-c1 |
| ORDERS_3       | File    |          | 5         | 0        | 0 B     | 0    | 0       | n1-c1, n2-c1*, n3-c1 |
| ORDERS_4       | File    |          | 5         | 0        | 0 B     | 0    | 0       | n1-c1*, n2-c1, n3-c1 |
| ORDERS_5       | File    |          | 5         | 0        | 0 B     | 0    | 0       | n1-c1*, n2-c1, n3-c1 |
| ORDERS_8       | File    |          | 5         | 0        | 0 B     | 0    | 0       | n1-c1, n2-c1*, n3-c1 |
| ORDERS_9       | File    |          | 5         | 0        | 0 B     | 0    | 0       | n1-c1, n2-c1*, n3-c1 |
| ORDERS_0       | File    |          | 6         | 100      | 4.5 KiB | 0    | 0       | n1-c1, n2-c1*, n3-c1 |
| ORDERS_1       | File    |          | 6         | 100      | 4.5 KiB | 0    | 0       | n1-c1*, n2-c1, n3-c1 |
| ORDERS_ARCHIVE | File    |          | 0         | 200      | 27 KiB  | 0    | 0       | n2-c2*               |
+----------------+---------+----------+-----------+----------+---------+------+---------+----------------------+```

Worst case since my messages are tiny though

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After our discussion let me know if you still want me to look into shrinking or do we think this could be valuable as is to end users?

/cc @kozlovic

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think its ok, it should be useful. 1 billion messages = 70GB of this header :P

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compression ;)

Copy link
Member

@kozlovic kozlovic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

server/raft.go Show resolved Hide resolved
server/raft.go Show resolved Hide resolved
.travis.yml Show resolved Hide resolved
Signed-off-by: Derek Collison <derek@nats.io>
derekcollison and others added 4 commits February 23, 2021 17:20
Signed-off-by: Derek Collison <derek@nats.io>
Signed-off-by: Derek Collison <derek@nats.io>
Signed-off-by: Derek Collison <derek@nats.io>
Remove panic on runAsLeader when not able to subscribe (which happens
on shutdown)
Gateway name access does not need lock since it is immutable. Will
prevent deadlocks in some situations.

Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
@derekcollison derekcollison merged commit bec42c0 into master Feb 24, 2021
@derekcollison derekcollison deleted the mirrors2 branch February 24, 2021 02:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants