2 commits

Author	SHA1	Message	Date
Travis Turner	2843f218bc	Introduce ServiceManager and Refactor DAX Integration tests (#2320) ... * Introduce ServiceManager and Refactor DAX Integration tests The ServiceManager provides an interface with which to manage featurebase (dax) services (mds, queryer, computer). It replaces the confusing interface implementations in /dax/server/server.go (which optionally used pointers to in-process objects to satisfy an interface) with (for now) http implementations. The thought is that even if we're running all services in-process, we should communicate between services over http in order to mirror what we would do in a production environment where the services are running on different nodes. This batch of commits does quit a lot, most of which is captured here: - Added `path` support to `dax.Address`. Address is now a string of the form [scheme]://[host]:[port]/[path]. - Added `Holder.directiveApplied` to determine (in tests) if the computer has completed applying the latest directive. This is somewhat temporary until we improve the mds-to-computer logic. - Removed the "service prefix" code which was prepending client URL paths with the prefix. Instead, the serviceType (mds, queryer, computer[n] is now part of `dax.Address`). - Removed, from the dax config, the top level `StorageMethod` and `StorageDSN` and now just have `MDS.Config.DataDir`. - Added `Computer.Config.N` to specify the number of computers to run in-process. - Moved the `pilosa.MDS` interface to `computer.Registrar`. This is an example of getting the interfaces defined in the right packages. - Added `SnapshotTable()` method to the mds client (to align with its API). - Changed `Balancer.AddJob()` to `Balancer.AddJobs()` to support, for example, adding 256 partitions in a single call. Refactored some of the naive Balancer to account for this. - Added a `Seed` to the top-level config. It's not really useful because of package `crypto/rand`. - Added an in-memory implementation of the DisCo interface and disabled etcd in a computer service. - Create sepearte data-dirs for each in-process computer. - Disabled grpc in dax. - Modified the sql3 test definition format to support multiple insert steps and separate query results (to align with those steps). * Changes necessary to get multiple computer instance running in-process For now the config looks like this: ``` [computer] run = true n = 4 ``` but we can probably just change that to be something like: ``` [computer] run = 4 ``` *Issues found running multiple "computers" in-process* - grpc was trying to bind on the same port - changed GRPCListener from `*net.TCPListener` to `net.Listener` - created a nopListener and set to that for now (i.e. disabled grpc) - etcd was starting more than once - changed dax to use in-memory implementations of the disco interfaces (i.e. stop using etcd) - IDAllocator (which uses boltdb) was trying to open the `idalloc.db` file more than once - realized we have to set separate data-dirs for each holder. that fixed it. * Port dax integration tests to ManagedCommand * Modify Balancer-related methods like AddJob to AddJobs There were (and still are) a lot of places where we were adding on job at a time, even when we had a long list of jobs to add. This resulted in every job add (for example adding 1 of 256 shards) taking ~40ms, or over 10s to create a keyed table. One reason was because each job add was making multiple boltdb transactions. * Port over more dax integration test stuff * Add DirectiveApplied to signify that snapshot/writes have loaded. We use this in tests to avoid using sleeps. This should be considered temporary; we're going to need a more robust solution for determining when a computer node is ready to serve complete data. * Finish porting dax integration tests * Improve godocs * Remove docker-based DAX integration tests. * go mod tidy * Move test/managed.go to avoid package conflicts * Modify IDK integration tests to work with ServiceManager changes This is really just computer -> computer0 And the MDS DataDir config change. * cleanup found during review * echo $CI_COMMIT_REF_SLUG in CI * remove docker image arg, use build instead	2022-12-05 14:49:17 -06:00
Seebs	b3a4e52a13	simplify, streamline, and possibly debug embedded etcd ... The root problem this is attempting to address is sporadic weird cases in which etcd mistakenly thinks it's down even when it's up. I am not confident that this is addressed, but there's a reasonable chance that it is, and I can't trigger it at the moment, but it was always sporadic, so that doesn't prove much. There's a lot going on here, and it comes into roughly three categories. First: Dropping unused/unneeded code. There's a lot of leftover bits from the initial development and refactoring of this. Second: Unifying and shuffling some of the design. We had multiple interfaces which are functionally impossible to usefully implement separately, so they're combined together, and in some cases, moved. Third: Streamlining logic and simplifying design choices. This is combined into one commit because the changes are thoroughly entertwined with each other and you can't usefully break most of them out. Also, a bunch of test coverage for most of these changes. Big changes: We merge the topology and disco packages. The topology and disco packages being separate creates a complicated tangle of problems and dependencies. The fundamental problem, approximately, is that topology.Node has to track disco.NodeState. There's three core interfaces interacting here: topology.Noder (maintains list of nodes) disco.Stator (maintains the state of a node) disco.Metadator (stores, possibly retrieves, node metadata) But the node state mantained by the Noder *is* the set of node metadata, plus state updates produced by Stators. The only actual non-trivial and usable implementation of these interfaces is a single thing which implements all three, and in which the implementations share a single backend data source which they are all modifying. But you can't move Noder into disco, because Noder has to refer to topology.Node, but topology.Node refers to disco. Solution: First, merge these two packages. Second, merge these three interfaces, to provide a single interface which is more clear about the fact that (metadator.)SetMetadata() and (stator.)Started() are both changing the output we'll get from (noder.)Nodes(). We rework the node state tracking. We have this nodeStates map which is almost unused. Really, we don't need it at all. Every node's state is either its last heartbeat state or "Unknown", so we simplify this a bit. Also, we ensure that the populateNodeStates function itself is yielding the sorted nodes list, so we don't have to be as worried about possible later lookups of sortedNodes happening outside a lock. We also add diagnostics for deleting nodes from the metadata list (this should never happen), and try to track heartbeat state more closely. This is *probably* what fixes the underlying reported problem, if anything did. Still an open issue: Make heartbeat state changes aware of when they're talking about *this* node and possibly not try to mark it down? Except this may have a flaw: That would result in each node disagreeing with other nodes in etcd about the state of that node in the failure cases, and undermine the point of using etcd to keep these states consistent. We reduce the number of contexts and cancelfuncs in the etcd wrapper. We create a shared context for the non-etcd.embed children of our etcd wrapper, the heartbeat/keepalive and the node watcher, so we can cancel that one context and cancel all of those at once, so we don't need to separately track a function to call to cancel the watch, AND be closing another channel. Also, our shutdown now propagates automatically to the various etcd API calls we've made for things like the node watcher and keepalive calls. We still need to watch that channel in watchNodesOnce, though, because apparently the watch doesn't yield an error even if the context calling it is canceled. Whee. This should reduce the risk of ending up in an inconsistent state, and also the Close() function is probably idempotent now. Smaller changes: * Remove config-generators that existed to generate etcd configs but were used only for tests that no longer exist or make sense. * Move the logic to generate etcd configs into the etcd package, instead of the "testing" subpackage. This allows us to write a self-contained config generator for clusters where the nodes know about each other, but do this just with etcd, not with full featurebase servers. * Move the thing generating `fake:%d` socket names into the etcd package, which is the only place we use it. Also simplify it slightly. * Don't panic on invalid URLs, report errors from them. * At least try to use etcd's config.Validate functionality. It's underdocumented, so we're not sure what it will report, but at least if it does we'll get reports from it and know what they are? * Try to handle CompactRevision errors from watches more correctly -- after a CompactRevision, any future attempt to watch from a lower revision will necessarily fail, so we adjust our target revision up. We don't have good testing for this. * Drop the Metadata() method (that used to be in Metadator) because nothing ever used it and it didn't make much sense to try. * Convert SetMetadata from taking an arbitrary json blob to taking the only data that would ever be valid since we always use it to extract node information anyway. * Drop several unused functions, unexport things only used internally. * Replace Started() with SetState("STARTED"), allowing us to write tests that mess with states. We weren't really thinking carefully about state transitions sometimes and now it's much easier to do that thinking. * Stop leaving stray localhost:2380 and localhost:2379 in our embed config. We still sometimes see peer requests from those and I honestly don't know why, but at least it should be rarer.	2022-07-21 11:42:35 -05:00

Renamed from topology/noder.go (Browse further)