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

RESP 3 push model requires polling #1091

Closed
nihohit opened this issue Mar 21, 2024 · 4 comments · Fixed by #1096
Closed

RESP 3 push model requires polling #1091

nihohit opened this issue Mar 21, 2024 · 4 comments · Fixed by #1096

Comments

@nihohit
Copy link
Contributor

nihohit commented Mar 21, 2024

Currently sync & async connections only receive RESP3 disconnect push messages when a request is sent on the connection, because the incoming TCP traffic isn't polled independently. This means that for pubsub / monitor / client side caching works, but can't report issues - the user will only receive updates when they send on the message, not when the user is inactive.

@nihohit
Copy link
Contributor Author

nihohit commented Mar 21, 2024

@jaymell I think that this should be a blocker to the next release. Either that, or we hide the push manager behind a beta feature branch.

@altanozlu
Copy link
Contributor

It could only be a problem if user just subscribes to certain channels and then never subs/unsubs again or sending any commands.
It could be solved by a heartbeat PING command every x seconds, but I'm not sure if it's a blocker.

@jaymell
Copy link
Contributor

jaymell commented Mar 23, 2024

How are other clients handling this situation?

@altanozlu
Copy link
Contributor

I tried with rueidis and it also didn't gave an error when server is crashed, but we can fix this in multiplexed easily(tested it), with sync you always have to call get_message in pubsub, which should raise PushKind::Disconnection (didn't test it).

rueidis code:

client, err := rueidis.NewClient(rueidis.ClientOption{InitAddress: []string{"127.0.0.1:6379"}})
	if err != nil {
		panic(err)
	}
	defer client.Close()
	log.Println("connected")
	err = client.Receive(context.Background(), client.B().Subscribe().Channel("ch1", "ch2").Build(), func(msg rueidis.PubSubMessage) {
		log.Println(msg)
	})
	log.Println(err)

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 a pull request may close this issue.

3 participants