-
Notifications
You must be signed in to change notification settings - Fork 203
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
Update message table schema for efficient poller and w/o potential message loss #1015
Conversation
Signed-off-by: Matt Lord <mattalord@gmail.com>
✅ Deploy Preview for vitess ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
create table my_message(
# required columns
id bigint NOT NULL COMMENT 'often an event id, can also be auto-increment or a sequence',
priority tinyint NOT NULL DEFAULT '50' COMMENT 'lower number priorities process first',
epoch bigint NOT NULL DEFAULT '0' COMMENT 'Vitess increments this each time it sends a message, and is used for incremental backoff doubling',
time_next bigint DEFAULT 0 COMMENT 'the earliest time the message will be sent in epoch nanoseconds. Must be null if time_acked is set',
time_acked bigint DEFAULT NULL COMMENT 'the time the message was acked in epoch nanoseconds. Must be null if time_next is set',
# add as many custom fields here as required
# optional - these are suggestions
tenant_id bigint,
message json,
# required indexes
primary key(id),
index poller_idx(time_acked, priority, time_next desc)
# add any secondary indexes or foreign keys - no restrictions
) comment 'vitess_message,vt_min_backoff=30,vt_max_backoff=3600,vt_ack_wait=30,vt_purge_after=86400,vt_batch_size=10,vt_cache_size=10000,vt_poller_interval=30' |
See: vitessio/website#1015 Signed-off-by: Matt Lord <mattalord@gmail.com>
We don't have to address it in the PR, but I wonder if we could/should introduce a check constraint for anyone >= 8.0.16 that enforces the mutual exclusivity of |
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
@derekperkins could you please review the new page whenever you have time? I went through the entire thing and updated it. All of them are the same, so you can look at this one: https://deploy-preview-1015--vitess.netlify.app/docs/14.0/reference/features/messaging/ Thanks! |
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
|
||
# add as many custom fields here as required | ||
# optional - these are suggestions | ||
tenant_id bigint COMMENT 'offers a nice way to segment your messages', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we say "shard" instead of "segment"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like it could still be useful when not sharded. Some receivers could only process and ack messages for a given tenant. No?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have lots of unsharded queues - tenant id just feels like the most common sharding id, though I understand your point. My first reaction on reading it was to wonder if "segment" was a specific concept for messaging. I don't feel super strongly about it.
Once we're done with this, I want to blog about and/or add an opinionated design to the docs, with the specific fields and queries we use, so people don't have to architect from scratch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have lots of unsharded queues - tenant id just feels like the most common sharding id, though I understand your point. My first reaction on reading it was to wonder if "segment" was a specific concept for messaging. I don't feel super strongly about it.
I don't have a strong opinion here either. We can always revisit.
Once we're done with this, I want to blog about and/or add an opinionated design to the docs, with the specific fields and queries we use, so people don't have to architect from scratch.
That would be awesome! ❤️
For a message that was successfully sent we will wait for the specified `vt_ack_wait` | ||
time. If no ack is received by then, it will be resent. The next attempt will be 2x | ||
the previous wait, and this delay is doubled for every attempt (with some added | ||
jitter to avoid thundering herds). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should probably add details for vt_min_backoff
and vt_max_backoff
here.
Also, maybe we should specify that "some added jitter" is "up to 33% jitter"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if we want to get this technical, but here's a first stab at giving actual details
Basic backoff equation:
jitteredBackoff = `vt_ack_wait` ^ `epoch` * (random value between 0.66 and 1.33)
actualBackoff = `vt_min_backoff` <= `jitteredBackoff` <= `vt_max_backoff`
backoff query if vt_max_backoff
is NOT set
update foo set time_next =
:time_now + :wait_time + IF(FLOOR((:min_backoff<<ifnull(epoch, 0)) * :jitter) < :min_backoff, :min_backoff, FLOOR((:min_backoff<<ifnull(epoch, 0)) * :jitter)), epoch = ifnull(epoch, 0)+1 where id in ::ids and time_acked is null
backoff query if vt_max_backoff
IS set
update foo set time_next =
:time_now + :wait_time + IF(FLOOR((:min_backoff<<ifnull(epoch, 0)) * :jitter) < :min_backoff, :min_backoff, IF(FLOOR((:min_backoff<<ifnull(epoch, 0)) * :jitter) > :max_backoff, :max_backoff, FLOOR((:min_backoff<<ifnull(epoch, 0)) * :jitter))), epoch = ifnull(epoch, 0)+1 where id in ::ids and time_acked is null
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI, this is definitely one place where having to handle null epoch adds complexity. If we're ok enforcing non-null epoch, we could change this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rather leave this as it is for now (we're already making a lot of changes) and not share this level of detail in the docs. For those interested, the source code is there.
* Changed properties: Although the engine detects new message tables, it does | ||
not refresh properties of an existing table. | ||
* A `SELECT` style syntax for subscribing to messages. | ||
* No rate limiting. | ||
* Usage of partitions for efficient purging. | ||
not refresh the properties (such as `vt_ack_wait`) of an existing table. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is true anymore, but I'm not sure
Adding |
Signed-off-by: Matt Lord <mattalord@gmail.com>
* Prevent deadlocks related to 0 receiver behavior Signed-off-by: Matt Lord <mattalord@gmail.com> * Update test tables to use poller_idx Signed-off-by: Matt Lord <mattalord@gmail.com> * Minor changes after mutex usage review in message manager + cache Signed-off-by: Matt Lord <mattalord@gmail.com> * Use atomics for receiver count and messages pending Signed-off-by: Matt Lord <mattalord@gmail.com> * Don't take exclusive lock when in fast path Signed-off-by: Matt Lord <mattalord@gmail.com> * Update tests to use the new recommended message table structure See: vitessio/website#1015 Signed-off-by: Matt Lord <mattalord@gmail.com> * Correct tests Signed-off-by: Matt Lord <mattalord@gmail.com> * Update e2e test to use new recommended table structure Signed-off-by: Matt Lord <mattalord@gmail.com> * Fix TestMessageStreamingPlan test Signed-off-by: Matt Lord <mattalord@gmail.com> * Fix godriver/TestStreamMessaging test Signed-off-by: Matt Lord <mattalord@gmail.com> * Split streamMu into streamProcessingMu and lastPollPositionMu Signed-off-by: Matt Lord <mattalord@gmail.com> * Poller cannot take main lock w/o having X stream processing lock Signed-off-by: Matt Lord <mattalord@gmail.com> * Improve the comments a bit Signed-off-by: Matt Lord <mattalord@gmail.com> * Hold the main mutex during Add This is for safe concurrency with the last receiver unsubscribing Signed-off-by: Matt Lord <mattalord@gmail.com> * Changes after pair reviewing with Sugu Signed-off-by: Matt Lord <mattalord@gmail.com> * Use my GitHub handle for the self reference Signed-off-by: Matt Lord <mattalord@gmail.com>
* Prevent deadlocks related to 0 receiver behavior Signed-off-by: Matt Lord <mattalord@gmail.com> * Update test tables to use poller_idx Signed-off-by: Matt Lord <mattalord@gmail.com> * Minor changes after mutex usage review in message manager + cache Signed-off-by: Matt Lord <mattalord@gmail.com> * Use atomics for receiver count and messages pending Signed-off-by: Matt Lord <mattalord@gmail.com> * Don't take exclusive lock when in fast path Signed-off-by: Matt Lord <mattalord@gmail.com> * Update tests to use the new recommended message table structure See: vitessio/website#1015 Signed-off-by: Matt Lord <mattalord@gmail.com> * Correct tests Signed-off-by: Matt Lord <mattalord@gmail.com> * Update e2e test to use new recommended table structure Signed-off-by: Matt Lord <mattalord@gmail.com> * Fix TestMessageStreamingPlan test Signed-off-by: Matt Lord <mattalord@gmail.com> * Fix godriver/TestStreamMessaging test Signed-off-by: Matt Lord <mattalord@gmail.com> * Split streamMu into streamProcessingMu and lastPollPositionMu Signed-off-by: Matt Lord <mattalord@gmail.com> * Poller cannot take main lock w/o having X stream processing lock Signed-off-by: Matt Lord <mattalord@gmail.com> * Improve the comments a bit Signed-off-by: Matt Lord <mattalord@gmail.com> * Hold the main mutex during Add This is for safe concurrency with the last receiver unsubscribing Signed-off-by: Matt Lord <mattalord@gmail.com> * Changes after pair reviewing with Sugu Signed-off-by: Matt Lord <mattalord@gmail.com> * Use my GitHub handle for the self reference Signed-off-by: Matt Lord <mattalord@gmail.com> Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com>
…11619) * Move towards MySQL 8.0 as the default template generation (#11153) * Move towards MySQL 8.0 as the default template generation This upgrades the remaining things to Ubuntu 20.04 and makes MySQL 8.0 the default we run tests against. We still have tests for MySQL 5.7 but those are now explicitly opted into. This should finish up the Ubuntu 20.04 upgrade and also makes things easier for the future when we need to upgrade again. Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> * CI: rename shard vtorc_8.0 to vtorc_5.7, change expected test output for 8.0 Signed-off-by: deepthi <deepthi@planetscale.com> * CI: increase timeout for 8.0 tests on the actual test step from 30 to 45 mins Signed-off-by: deepthi <deepthi@planetscale.com> * CI: increase timeout to 45 minutes for mysql57 tests too. We really only need this for vtorc, but I've made the change to the template so all tests get it. Signed-off-by: deepthi <deepthi@planetscale.com> * CI: fix vtorc test to work with both 5.7 and 8.0 Signed-off-by: deepthi <deepthi@planetscale.com> * CI: missed docker flag in mysql57 template, one more fix to vtorc test Signed-off-by: deepthi <deepthi@planetscale.com> * removing spaces from pb file Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com> * removing spaces in pb file part 2 Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com> Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> Signed-off-by: deepthi <deepthi@planetscale.com> Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com> Co-authored-by: deepthi <deepthi@planetscale.com> Co-authored-by: Rameez Sajwani <rameezwazirali@hotmail.com> * fixing template files to default to mysql80 Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com> * changing update statement to alter Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com> * Avoid deadlocks related to 0 receiver behavior (#10132) * Prevent deadlocks related to 0 receiver behavior Signed-off-by: Matt Lord <mattalord@gmail.com> * Update test tables to use poller_idx Signed-off-by: Matt Lord <mattalord@gmail.com> * Minor changes after mutex usage review in message manager + cache Signed-off-by: Matt Lord <mattalord@gmail.com> * Use atomics for receiver count and messages pending Signed-off-by: Matt Lord <mattalord@gmail.com> * Don't take exclusive lock when in fast path Signed-off-by: Matt Lord <mattalord@gmail.com> * Update tests to use the new recommended message table structure See: vitessio/website#1015 Signed-off-by: Matt Lord <mattalord@gmail.com> * Correct tests Signed-off-by: Matt Lord <mattalord@gmail.com> * Update e2e test to use new recommended table structure Signed-off-by: Matt Lord <mattalord@gmail.com> * Fix TestMessageStreamingPlan test Signed-off-by: Matt Lord <mattalord@gmail.com> * Fix godriver/TestStreamMessaging test Signed-off-by: Matt Lord <mattalord@gmail.com> * Split streamMu into streamProcessingMu and lastPollPositionMu Signed-off-by: Matt Lord <mattalord@gmail.com> * Poller cannot take main lock w/o having X stream processing lock Signed-off-by: Matt Lord <mattalord@gmail.com> * Improve the comments a bit Signed-off-by: Matt Lord <mattalord@gmail.com> * Hold the main mutex during Add This is for safe concurrency with the last receiver unsubscribing Signed-off-by: Matt Lord <mattalord@gmail.com> * Changes after pair reviewing with Sugu Signed-off-by: Matt Lord <mattalord@gmail.com> * Use my GitHub handle for the self reference Signed-off-by: Matt Lord <mattalord@gmail.com> Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com> * remove unwanted sleep Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com> * Fix failures Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com> * Fix backup tests Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com> * fix vtgate_gen4 error Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com> * upgrading to mysql 8.0 using vinalla mysql Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com> * pin to specific version 8.0.25 Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com> * Try to pin mysql version to 8.0.25 using tar file Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com> * Fix bug in last commit for 8.0.25 Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com> * fixing template files to use mysql8.0.25 Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com> * fixing community version Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com> * removing all mysql version before installation Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com> * moving cluster 11 to self host Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com> * trying different combination since mysql is not getting installed correctly Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com> * use tar instead of deb file Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com> * remove mysql stop statement Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com> * setting vt_mysql_root Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com> * moving export to right place Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com> * change template to accomodate tar file download Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com> * fixing mysql80 cluster Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com> * Adding mysql to the path env variable Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com> * fixing shardedpitr_tls test Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com> * fix upgrade-downgrade test Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com> * adjust epected AUTO_INCREMENT value Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * removed 'expected_table_structure' files because there are different outputs in mysql57 and in mysql80 Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> * adding mysql version in workflow logs Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com> * move to utuntu 18 for upgrade-downgrade test Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com> Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com> Signed-off-by: deepthi <deepthi@planetscale.com> Signed-off-by: Rameez Sajwani <rameezwazirali@hotmail.com> Signed-off-by: Matt Lord <mattalord@gmail.com> Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Co-authored-by: Dirkjan Bussink <d.bussink@gmail.com> Co-authored-by: deepthi <deepthi@planetscale.com> Co-authored-by: Matt Lord <mattalord@gmail.com> Co-authored-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>
This updates the docs based on the work done in: vitessio/vitess#10132
The potential for loss of messages — or more accurately messages never sent — comes from the
time_next bigint
column definition that we had in the docs, which is effectivelytime_next bigint DEFAULT NULL
. If thetime_next
value is NULL, then it never gets read from the underlying message table in the poller / results streamer query because<
is not NULL safe and that< <time_next_val>
clause will never match those rows where the value is NULL (unknown). So while rows INSERTed withtime_next
being NULL while the binlog streamer is running will catch these rows and process them, any rows INSERTed while the binlog streamer is not running, or the cache is full, will be lost.We were already using a
DEFAULT 0
clause for thetime_next
column in our tests, but this was not done in the docs.