Combine MutationStore2, Collection, DataStoreName into DataStore #6752
Conversation
Redocly previews |
This is cleanup from the introduction to rosmar. - rosmar.Collection and Collection already satisified the DataStoreName interface, so putting it as part of DataStore removes need for AsDataStoreName checks. - remove GetFeedType() checks since it will always return DCP - Remove StartTapFeed from leaky bucket because it was not being used. TestStopChangeCache was only running with xattrs disabled, but the test would pass even if the events arrived out of order, so the leaky bucket tap code was not being executed. - Reflect in documentation that DCP feed is always used.
torcolvin
force-pushed
the
simplify_interfaces
branch
from
e23ab16
to
4113896
Compare
| // will be filtered such that only the _latest_ mutation will make it through. | ||
| TapFeedDeDuplication bool | ||
| TapFeedVbuckets bool // Emulate vbucket numbers on feed | ||
| TapFeedMissingDocs []string // Emulate entry not appearing on tap feed |
There was a problem hiding this comment.
TapFeedMissingDocs doesn't seem like tap vs. DCP functionality, it's more about tests having the ability to simulate a document not arriving over the feed. I understand we don't have any real test usage of this today - are you thinking we'd just rebuild if/when it's needed?
There was a problem hiding this comment.
I was thinking we'd rebuild it when we needed it. I'm OK leaving this code in, and I think that it could be used to build artificial DCP feeds.
db/change_cache_test.go
Outdated
| @@ -1299,44 +1299,6 @@ func TestChannelRace(t *testing.T) { | |||
| changesCtxCancel() | |||
| } | |||
|
|
|||
| // Test that housekeeping goroutines get terminated when change cache is stopped | |||
| func TestStopChangeCache(t *testing.T) { | |||
There was a problem hiding this comment.
This seems like a valid test case - do you think this scenario has coverage elsewhere?
There was a problem hiding this comment.
I think that the test code can be left in, but it wasn't using the leaky tap feed. I am happy to do that.
| @@ -278,15 +244,6 @@ func (listener *changeListener) NotifyCheckForTermination(ctx context.Context, k | |||
| listener.tapNotifier.L.Unlock() | |||
| } | |||
|
|
|||
| func (listener *changeListener) notifyStopping(ctx context.Context) { | |||
There was a problem hiding this comment.
This doesn't seem like it should be tap-specific. Are we sure it's not an oversight that we don't handle this in the DCP case?
There was a problem hiding this comment.
I also wondered about this if there is a latent bug here (this code is unused). I think can affect things like the change feed not exiting because nothing goes to wake up the sync.Cond - so in changes.go changeWaiter.Wait() may not get woken up.
This code is not used today though.
| @@ -1273,7 +1273,7 @@ Database: | |||
| db_state_changed: | |||
| $ref: '#/Event-config' | |||
| feed_type: | |||
| description: The type of feed to use to communicate with Couchbase Server. | |||
| description: The type of feed to use to communicate with Couchbase Server. This will use DCP regardless of specification. | |||
There was a problem hiding this comment.
Are we required to keep TAP in the enum for backward compatibility, or could it be removed.
There was a problem hiding this comment.
I can definitely just remove this.
* Combine MutationStore2, Collection, DataStoreName into DataStore This is cleanup from the introduction to rosmar. - rosmar.Collection and Collection already satisified the DataStoreName interface, so putting it as part of DataStore removes need for AsDataStoreName checks. - remove GetFeedType() checks since it will always return DCP - Remove StartTapFeed from leaky bucket because it was not being used. TestStopChangeCache was only running with xattrs disabled, but the test would pass even if the events arrived out of order, so the leaky bucket tap code was not being executed. - Reflect in documentation that DCP feed is always used. * Replace leaky bucket code for DCPMissingDocs
This is cleanup from the introduction to rosmar.
rosmar.Collection and Collection already satisified the DataStoreName interface, so putting it as part of DataStore removes need for AsDataStoreName checks.
remove GetFeedType() checks since it will always return DCP
Remove StartTapFeed from leaky bucket because it was not being used. TestStopChangeCache was only running with xattrs disabled, but the test would pass even if the events arrived out of order, so the leaky bucket tap code was not being executed.
Reflect in documentation that DCP feed is always used.
I could leave the code from the leaky bucket tap feed to resurrect it when we need it, it did "work" in rosmar but it wasn't testing anything. I think knowing that it exists to look back at the implementation when it is needed is important, especially since it does not necessarily do what it claims to do.
couchbaselabs/rosmar#30
couchbase/sg-bucket#119
Pre-review checklist
fmt.Print,log.Print, ...)base.UD(docID),base.MD(dbName))docs/apiDependencies (if applicable)
Integration Tests
GSI=true,xattrs=truehttps://jenkins.sgwdev.com/job/SyncGateway-Integration/2371/