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

RFC: Messager simplification -> Breaking change #5947

Closed
sougou opened this issue Mar 21, 2020 · 2 comments · Fixed by #5948
Closed

RFC: Messager simplification -> Breaking change #5947

sougou opened this issue Mar 21, 2020 · 2 comments · Fixed by #5948

Comments

@sougou
Copy link
Contributor

sougou commented Mar 21, 2020

After the move to RBR for messaging, and discussions with @derekperkins, we're simplifying the messaging API. On the upside, this will support all insert constructs. On the downside, the application has more responsibilities on correctness and features. What is changing:

  • time_created and time_scheduled are removed from the list of mandatory columns. They will neither be checked nor be populated. If necessary, the application can add these columns and keep them populated.
  • The primary key that used to be (time_scheduled,id) is now just id.
  • The unique key on id is not required any more, since the PK covers that.
  • time_next remains a nullable column, but it's recommended to default it to 0. Setting it to 0 (or any value < now) will schedule the message for immediate delivery. Previously a 0-value was treated as an acked message. This is not the case any more. This means that one could insert a row without specifying a time_next. The code checks for time_acked to contain a non-null (and non-zero) value to determine that a message was acked.
  • The default value for epoch can also be 0, but the code treats null values as 0 also.
  • Purging is by time_acked. Consequently, a new index is needed on time_acked.

Here's an idiomatic message table definition:

var createMessage = `create table vitess_message(
	id bigint,
	time_next bigint default 0,
	epoch bigint,
	time_acked bigint,
	message varchar(128),
	primary key(id),
	index next_idx(time_next, epoch),
	index ack_idx(time_acked))
comment 'vitess_message,vt_ack_wait=30,vt_purge_after=120,vt_batch_size=1,vt_cache_size=1000,vt_poller_interval=30'

Here's an idiomatic insert:

insert into vitess_message(id, message) values(1, 'hello world')
@derekperkins
Copy link
Member

For reference when we come back to document this, time_acked, and probably time_next, still aren't fully treating 0 as null. Setting time_acked to 0 results in immediate purging - starting with v6 and still true today.

https://github.com/vitessio/vitess/blob/main/go/vt/vttablet/tabletserver/messager/message_manager.go#L251-L258

@derekperkins
Copy link
Member

Verified that time_next has to be null, otherwise it gets picked up and continually resent. There must be some 0 handling because epoch doesn't get incremented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants