NServiceBus.SqlServer icon indicating copy to clipboard operation
NServiceBus.SqlServer copied to clipboard

Remove unused columns from queue table

Open tmasternak opened this issue 8 years ago • 5 comments

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:

  • Remove the Recoverable column. Details #72
  • Remove ReplyToAddress and CorrelationId columns. Details: #75

Plan of Action

  • [x] 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
    • PR to be released as part of SqlTransport v7, for NServiceBus 8: https://github.com/Particular/NServiceBus.SqlServer/pull/855
  • [ ] 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.

tmasternak avatar Mar 15 '16 12:03 tmasternak

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 avatar Jun 14 '21 17:06 DavidBoike

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 avatar Jun 14 '21 17:06 DavidBoike

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

https://github.com/Particular/NServiceBus.SqlServer/pull/855

DavidBoike avatar Jun 14 '21 21:06 DavidBoike

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

tmasternak avatar Jun 15 '21 07:06 tmasternak

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.

DavidBoike avatar Jun 22 '21 21:06 DavidBoike