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

Remove unused columns from queue table #179

Open
1 of 2 tasks
tmasternak opened this issue Mar 15, 2016 · 5 comments
Open
1 of 2 tasks

Remove unused columns from queue table #179

tmasternak opened this issue Mar 15, 2016 · 5 comments

Comments

@tmasternak
Copy link
Member

tmasternak commented Mar 15, 2016

Goal

The goal is to remove unused columns from queue table schema. The biggest problem here will probably be making sure we are backwards compatible with older versions of the transport.

Changes include:

Plan of Action

  • Phase 1: Make it so senders & receivers don't care whether the columns exist, but continue creating them in queue creation logic so that non-updated endpoints can send to them successfully
  • Phase 2: Remove columns from queue creation
    • This is a breaking change, cannot be released until SqlTransport v8
    • Upgrade guide must mention that all other endpoints in the system must be upgraded to at least SqlTransport v7 first. There will not be wire compatibility between v6 and v8.
@DavidBoike
Copy link
Member

Analyzing Recoverable

The problem here is that the Recoverable column has always been defined as Recoverable bit NOT NULL with no default, so SendText (the insert) must specify it.

If we remove the column, inserts will fail with an unknown column error. If we stop specifying the value, inserts will fail with no specified value for the column. If we do both, upgrades will fail becuase the table was created by a previous version. Even if the customer updates the table structure during upgrade of an endpoint, then other endpoints running a previous version won't be able to send messages to the upgraded endpoint becuase they'll be using the old insert statement that includes the column value.

Only solution that seems possible would be to have TableBasedQueue ask its table whether it has a Recoverable column when it is created, and then use a different command depending on the answer. Then once all endpoints were running at least that version, you could opt into a mode where you no longer create tables using that column, and optionally delete the column from existing tables, but because TableBasedQueue instances are cached for the life of an endpoint, removing the column from any given queue would require restarting any endpoint that might send to that queue.

@DavidBoike
Copy link
Member

Analyzing CorrelationId and ReplyToAddress

Unlike Recoverable, these two columns are nullable varchar(255), but are currently filled on insert with the value of a parameter, and returned to the result set on dequeue.

We can stop using the values on insert and on dequeue, but we can't remove them from the table or the table creation query because older versions sending messages to a new queue would get errors trying to insert a value into a non-existant column.

Once all endpoints in a system were updated to at least that version that no longer uses that information at insert/dequeue, a new version (likely a breaking change) could be introduced that no longer creates the columns.

@DavidBoike
Copy link
Member

Here's a spike of what it would take in the forthcoming major to remove the columns in the next major down the line.

#855

@tmasternak
Copy link
Member Author

@DavidBoike I think that making changes in #855 in the next major makes sense.

@DavidBoike
Copy link
Member

The changes in #855 have been merged and will be released along with NServiceBus 8.

Updated the issue description with breadcrumbs for what needs to happen in (no earlier than) Sql Transport version 8, which could still target NServiceBus 8.

That's all we can do for now.

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

No branches or pull requests

2 participants