CBG-4008 Support all config updates during async initialization #6909
Conversation
When using persistent config, all config updates (local or remote) should support async initialization, and not block on index readiness.
|
Integration test failure (TestResyncWithoutIndexes) is unrelated to this change. |
Until I realized this test was not even in this branch, I thought it could be related to the idea that the test doesn't delete the indexes correctly. Then I remembered that unlike |
| keyspace := dbName | ||
| if len(dbConfig.Scopes) > 0 { | ||
| keyspaces := getRESTKeyspaces(dbName, dbConfig.Scopes) | ||
| keyspace = keyspaces[0] | ||
| } |
There was a problem hiding this comment.
| keyspace := dbName | |
| if len(dbConfig.Scopes) > 0 { | |
| keyspaces := getRESTKeyspaces(dbName, dbConfig.Scopes) | |
| keyspace = keyspaces[0] | |
| } | |
| keyspace := getRESTKeyspaces(dbName, dbConfig.Scopes)[0] |
This is always > 0, in fact it is always 3.
There was a problem hiding this comment.
Do you mean this is always 1 when base.TestRequiresCollections(t) passes? I don't think it's ever three in my local testing. I expect this code (copied from other tests) is just defensive.
There was a problem hiding this comment.
I think it is be three because dbConfig := makeDbConfig(t, tb, syncFunc, importFilter) creates 3 collections always.
| sc, closeFn := rest.StartBootstrapServer(t) | ||
| defer closeFn() | ||
|
|
||
| // Set testing callbacks for async initialization |
There was a problem hiding this comment.
| // Set testing callbacks for async initialization | |
| // Set testing callbacks for async initialization to pause after a single collection's indexes have been created. |
We could actually shorten this test timeout to only create a single named collection and speed up these tests, since it will loop once for named collection and the default collection. This can make a pretty significant speedup over a bunch of tests, see #6911
There was a problem hiding this comment.
Do you think this merits this being handled differently than the rest of our test suite (i.e. using SG_TEST_COLLECTION_POOL_SIZE)?
| // Attempt to update import filter while in starting mode | ||
| importFilter = "function(doc){ return false; }" | ||
| resp := rest.BootstrapAdminRequest(t, sc, http.MethodPut, "/"+keyspace+"/_config/import_filter", importFilter) | ||
| resp.RequireStatus(http.StatusOK) | ||
|
|
||
| resp = rest.BootstrapAdminRequest(t, sc, http.MethodGet, "/"+keyspace+"/_config/import_filter", "") | ||
| resp.RequireResponse(http.StatusOK, importFilter) |
There was a problem hiding this comment.
I'd prefer to make sure we don't miss steps here, and put the test cases for sync and import_filter and make sure Put/Get/Delete/Get workflow works. This could be pulled out from the code above.
There was a problem hiding this comment.
What steps are you thinking we're missing here? This test (TestAsyncInitRemoteConfigUpdates) is ensuring that async works correctly when the db is updated on a remote nod, while the previous test (TestAsyncInitConfigUpdates) validates all the different config update options locally. I included the import filter change here to make sure the database wasn't in a state that would reject changes after processing a remote update. Beyond that, I'd expect the coverage in TestAsyncInitRemoteConfigUpdates to suffice.
There was a problem hiding this comment.
I think we can drop this, but I thought we'd just be complete to keep import and sync. I do think that the combination of tests covers all cases.
Target the correct import filter when making non-local update to config in TestAsyncInitRemoteConfigUpdates. Modify the way the existing TestAsyncInitWithResync drops buckets to avoid interference with the existing TestResyncWithoutIndexes.
* CBG-4008 Support all config updates during async initialization When using persistent config, all config updates (local or remote) should support async initialization, and not block on index readiness. * lint fixes * Test fixes Target the correct import filter when making non-local update to config in TestAsyncInitRemoteConfigUpdates. Modify the way the existing TestAsyncInitWithResync drops buckets to avoid interference with the existing TestResyncWithoutIndexes. * lint fix
When using persistent config, all config updates (local or remote) should support async initialization, and not block on index readiness.
CBG-4008
Integration Tests
GSI=true,xattrs=truehttps://jenkins.sgwdev.com/job/SyncGateway-Integration/2535/