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

libs/pubsub: make message tags available to subscribers #1879

Closed
zramsay opened this issue Jul 3, 2018 · 2 comments
Closed

libs/pubsub: make message tags available to subscribers #1879

zramsay opened this issue Jul 3, 2018 · 2 comments
Labels
C:libs Component: Library C:light Component: Light good first issue Contributions Welcome!!
Milestone

Comments

@zramsay
Copy link
Contributor

zramsay commented Jul 3, 2018

original issue: tendermint/tmlibs#143

see ^ for lengthy discussion

@zramsay zramsay added the C:libs Component: Library label Jul 3, 2018
@xla xla added C:light Component: Light good first issue Contributions Welcome!! labels Jul 8, 2018
@xla xla added this to the launch milestone Jul 8, 2018
@melekes
Copy link
Contributor

melekes commented Oct 2, 2018

We definitely want to send tags alongside the msg. The first "obvious solution" seems like a good way to go.

A solution I like better is to drop the message/tag distinction and make a message a key-value map, as if everything were a tag:

I don't see how this plays with the current query specification (e.g. "atom_count > 5").

Publish(ctx context.Context, keyvals... interface{}) error

I have a msg ("Hi") and atom_count: 6. Of course, we can encode the latter as a string atom_count=6, but that would require parsing (before matching) and not really efficient.

@Bric3d still want to help?

melekes added a commit that referenced this issue Oct 2, 2018
melekes added a commit that referenced this issue Oct 4, 2018
melekes added a commit that referenced this issue Oct 4, 2018
melekes added a commit that referenced this issue Nov 1, 2018
melekes added a commit that referenced this issue Nov 2, 2018
* pubsub adr

Refs #951, #1879, #1880

* highlight question

* fix typos after Ismail's review
@melekes
Copy link
Contributor

melekes commented May 6, 2019

Done in #3227.

@melekes melekes closed this as completed May 6, 2019
firelizzard18 pushed a commit to AccumulateNetwork/tendermint that referenced this issue Feb 1, 2024
…int#1879)

* blocksync: wait for poolRoutine to stop in (*Reactor).OnStop

blocksync.(*Reactor).poolRoutine goroutine lifetime was not supervised,
so OnStop returning did not provide a guarantee that it was done.  As a
result, the other services could be stopped and a subsequent call to
bcR.blockExec.ApplyBlock would cause a panic, typically a leveldb
closed error.

This change uses a sync.WaitGroup to ensure that OnStop returns after
poolRoutine has returned.

This also triggers the poolRoutine method to return on a signal from
bcR.pool.Quit() instead of just bcR.Quit(), which seems to be important
as (*Reactor).OnStop itself can only stop the BlockPool (bcR.pool), while
the BaseReactor.BaseService's quit channel will only be closed _after_
(*Reactor).OnStop has returned.

* rename wg field, comment on strayish select before retry

* break poolRoutine on either Quit before quick retry

* update changelog

* break the loop with an IsRunning check

* do the waitgroup Add and Defer at the same site for clarity

* check both services IsRunning in poolRoutine

* fix oopsie

---------

Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:libs Component: Library C:light Component: Light good first issue Contributions Welcome!!
Projects
None yet
Development

No branches or pull requests

3 participants