From 0eba050054aa260cb780d3dab7f67c5f89c77c5a Mon Sep 17 00:00:00 2001 From: Seebs Date: Mon, 2 Dec 2019 13:42:51 -0600 Subject: [PATCH] stop using pkg/plugin, start using build tags After a few experiments with pkg/plugin, I'm ready to concede that the people warning me it was unsuitable for production use were in fact correct. In the brave new world, the "ext" package is moved to its own module outside pilosa. This means that importing it doesn't imply any need to version-check against pilosa; we can just use versioned copies of the ext package, which can be public because it doesn't contain anything we need to care about keeping proprietary. Then we can, conditional on build tags, import modules from a neighboring repo which contains the actual implementations, and if they're imported, their init functions register them. --- executor.go | 2 +- ext/ext.go | 263 --------------------- ext/samples/.gitignore | 1 - ext/samples/some/some.go | 125 ---------- extension.go | 2 +- {ext/extensions => extensions}/distinct.go | 0 {ext/extensions => extensions}/dummy.go | 0 pql/ast.go | 2 +- row.go | 2 +- server.go | 4 +- 10 files changed, 6 insertions(+), 395 deletions(-) delete mode 100644 ext/ext.go delete mode 100644 ext/samples/.gitignore delete mode 100644 ext/samples/some/some.go rename {ext/extensions => extensions}/distinct.go (100%) rename {ext/extensions => extensions}/dummy.go (100%) diff --git a/executor.go b/executor.go index 47c08043b..f9586f958 100644 --- a/executor.go +++ b/executor.go @@ -23,7 +23,7 @@ import ( "sync" "time" - "github.com/pilosa/pilosa/v2/ext" + "github.com/molecula/ext" "github.com/pilosa/pilosa/v2/pql" "github.com/pilosa/pilosa/v2/roaring" "github.com/pilosa/pilosa/v2/shardwidth" diff --git a/ext/ext.go b/ext/ext.go deleted file mode 100644 index 762ae44f8..000000000 --- a/ext/ext.go +++ /dev/null @@ -1,263 +0,0 @@ -// Copyright 2019 Pilosa Corp. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -// Package ext provides an EXPERIMENTAL AND TEMPORARY interface to use for -// plugin extensions to Pilosa. DO NOT DEVELOP NEW PLUGINS WITH THIS. The -// replacement design is already in process, but it needs more refinement -// to address issues. This one has those issues, and more. -// -// In the current design, plugins will be loaded at runtime using the -// go `plugin` package, so they should be built as a main package using -// the plugin build mode. -// -// Plugins should not import other packages from Pilosa. -// -// To advertise their functionality, plugins define one or more of a -// handful of symbols which will be checked for at plugin load and used -// to register their functionality. -// -// The plugin interface will check for the following function(s). If the -// functions exist, they must have the given signatures. If they return -// a non-nil error, no ops are registered, and the error message will -// be reported in the Pilosa server's logs. -// -// BitmapOps() ([]BitmapOp, error) -// -// These functions may be absent, and may return nil slices; in either -// case, no ops are registered. -package ext - -import "sync" - -// The Bitmap type represents a Pilosa bitmap, and is used for bitmap -// operations. -type Bitmap interface { - // AddN and RemoveN can be used to add or remove values from a bitmap. - AddN(a ...uint64) (int, error) - RemoveN(a ...uint64) (int, error) - - // Lookups - Max() uint64 - Min() (uint64, bool) - Count() uint64 - Any() bool - Contains(uint64) bool - Slice() []uint64 - SliceRange(uint64, uint64) []uint64 - // ContainerBits stores the next 1<<16 bits, starting at the provided - // bit index. It may use a provided []uint64 to store them, or may - // provide its own. Don't write to those bits. Offset must be a multiple - // of 1<<16. - ContainerBits(uint64, []uint64) []uint64 - - // These operators provide existing implemented binary ops. - Intersect(Bitmap) Bitmap - Union(Bitmap) Bitmap - IntersectionCount(Bitmap) uint64 - Difference(Bitmap) Bitmap - Xor(Bitmap) Bitmap - Shift(int) (Bitmap, error) - Flip(uint64, uint64) Bitmap - - // New() is an atrocity: it creates a new bitmap, unrelated to the - // existing bitmap. This lets you create a new bitmap without having - // imported any of the packages that have bitmap creation tools, because - // the bitmap wrapper type has to give you one. - New() Bitmap -} - -// SignedBitmap represents a bitmap that can contain both positive and negative -// values. -type SignedBitmap struct { - Pos, Neg Bitmap -} - -// A BitmapOp represents a new bitmap operation that should be exposed -// in PQL. - -type BitmapOpInput byte -type BitmapOpOutput byte -type BitmapOpArity byte -type BitmapOpPrecall byte -type BitmapOpType struct { - Input BitmapOpInput - Arity BitmapOpArity - Output BitmapOpOutput - Precall BitmapOpPrecall -} - -const ( - OpArityUnary = BitmapOpArity(iota) - OpArityBinary - OpArityNary -) - -const ( - // Unary: Exactly one bitmap. - OpInputBitmap = BitmapOpInput(iota) - // The really weird special case used for BSI, where we end up - // needing to do BSI computations. Arguments will be a - // single BitmapBSI, and a []Bitmap for other operands if any. - OpInputNaryBSI -) - -const ( - OpOutputCount = BitmapOpOutput(iota) - OpOutputBitmap - OpOutputSignedBitmap -) - -const ( - OpPrecallNone = BitmapOpPrecall(iota) - OpPrecallGlobal - OpPrecallLocal // unimplemented -) - -// Regardless of arity, non-BSI functions should always take []Bitmap. -type BitmapOpFunc interface { - BitmapOpType() BitmapOpType -} - -// BitmapOpBitmap should actually always be func([]Bitmap) Bitmap, but -// might be different kinds. -type BitmapOpBitmap interface { - BitmapOpArity() BitmapOpArity - BitmapOpFunc() GenericBitmapOpBitmap -} - -// the common underlying type of the other BitmapOpBitmap functions -type GenericBitmapOpBitmap func([]Bitmap, map[string]interface{}) Bitmap - -// BitmapBSI represents the way a single BSI field is passed into a function -// which takes a BSI field. -type BitmapBSI struct { - FieldData Bitmap - ShardWidth uint64 - Offset int64 - Depth uint -} - -type BitmapOpBSIBitmap func(BitmapBSI, []Bitmap, map[string]interface{}) SignedBitmap - -func (b BitmapOpBSIBitmap) BitmapOpType() BitmapOpType { - return BitmapOpType{Input: OpInputNaryBSI, Arity: OpArityNary, Output: OpOutputSignedBitmap} -} - -type BitmapOpBSIBitmapPrecall func(BitmapBSI, []Bitmap, map[string]interface{}) SignedBitmap - -func (b BitmapOpBSIBitmapPrecall) BitmapOpType() BitmapOpType { - return BitmapOpType{Input: OpInputNaryBSI, Arity: OpArityNary, Precall: OpPrecallGlobal, Output: OpOutputSignedBitmap} -} - -type BitmapOpUnaryCount func([]Bitmap, map[string]interface{}) int64 - -func (b BitmapOpUnaryCount) BitmapOpType() BitmapOpType { - return BitmapOpType{Input: OpInputBitmap, Arity: OpArityUnary, Output: OpOutputCount} -} - -type BitmapOpUnaryBitmap func([]Bitmap, map[string]interface{}) Bitmap - -func (b BitmapOpUnaryBitmap) BitmapOpType() BitmapOpType { - return BitmapOpType{Input: OpInputBitmap, Arity: OpArityUnary, Output: OpOutputBitmap} -} - -func (b BitmapOpUnaryBitmap) BitmapOpArity() BitmapOpArity { - return OpArityUnary -} - -func (b BitmapOpUnaryBitmap) BitmapOpFunc() GenericBitmapOpBitmap { - return GenericBitmapOpBitmap(b) -} - -type BitmapOpBinaryBitmap func([]Bitmap, map[string]interface{}) Bitmap - -func (b BitmapOpBinaryBitmap) BitmapOpType() BitmapOpType { - return BitmapOpType{Input: OpInputBitmap, Arity: OpArityBinary, Output: OpOutputBitmap} -} - -func (b BitmapOpBinaryBitmap) BitmapOpArity() BitmapOpArity { - return OpArityBinary -} - -func (b BitmapOpBinaryBitmap) BitmapOpFunc() GenericBitmapOpBitmap { - return GenericBitmapOpBitmap(b) -} - -type BitmapOpNaryBitmap func([]Bitmap, map[string]interface{}) Bitmap - -func (b BitmapOpNaryBitmap) BitmapOpType() BitmapOpType { - return BitmapOpType{Input: OpInputBitmap, Arity: OpArityNary, Output: OpOutputBitmap} -} - -func (b BitmapOpNaryBitmap) BitmapOpArity() BitmapOpArity { - return OpArityNary -} - -func (b BitmapOpNaryBitmap) BitmapOpFunc() GenericBitmapOpBitmap { - return GenericBitmapOpBitmap(b) -} - -// BitmapOp represents an operation to be supported in PQL. Operations -// on bitmaps should always take []Bitmap. Operations on InputNaryBSI should -// take a []Bitmap, plus a Bitmap/shard-width/offset/depth. -// -// Reserved is a list of words to treat as reserved words in a prototype. -// This is not currently used but might be later, and I want to have the -// concept handy now. -type BitmapOp struct { - Name string - Func BitmapOpFunc - Reserved []string -} - -// ExtensionInfo tells us about the extension. The ExtensionAPI string -// should be "v0". The version is a human-readable version, use something -// that seems meaningful. Name and Description are reasonably self-explanatory, -// I hope. -// -// Extensions should define a function: -// func ExtensionInfo(extensionAPI string) (*ExtensionInfo, error) -// which reports their extension info if they think they can coexist with that -// API string. -type ExtensionInfo struct { - Name string // Extension name. - Description string // Short description. - Version string // Human-readable version info for extension. - ExtensionAPI string // Extension API version. Should be v0 for now. - License string // License info. - BitmapOps []BitmapOp // List of provided ops. -} - -var extMu sync.Mutex - -var knownExtensions []*ExtensionInfo -var newExtensions []*ExtensionInfo - -// RegisterExtension tells the extension system about a new extension. -func RegisterExtension(ext *ExtensionInfo) { - extMu.Lock() - defer extMu.Unlock() - newExtensions = append(newExtensions, ext) -} - -// NewExtentsions returns extensions that have been registered, but not previously -// returned by Newextensions. -func NewExtensions() []*ExtensionInfo { - extMu.Lock() - defer extMu.Unlock() - knownExtensions = append(knownExtensions, newExtensions...) - ret := newExtensions - newExtensions = nil - return ret -} diff --git a/ext/samples/.gitignore b/ext/samples/.gitignore deleted file mode 100644 index a63fa2c94..000000000 --- a/ext/samples/.gitignore +++ /dev/null @@ -1 +0,0 @@ -*/*.so diff --git a/ext/samples/some/some.go b/ext/samples/some/some.go deleted file mode 100644 index 07088d255..000000000 --- a/ext/samples/some/some.go +++ /dev/null @@ -1,125 +0,0 @@ -// Copyright 2019 Pilosa Corp. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package main - -import ( - "fmt" - "math/bits" - - "github.com/molecula/apophenia" - "github.com/pilosa/pilosa/v2/ext" -) - -// This could be dynamically generated, but for now it's not. -// nolint:unused,deadcode -var extInfoTemplate = &ext.ExtensionInfo{ - Name: "some", - Description: "some of the bits/all of the bits/none of the bits", - Version: "0.01", - ExtensionAPI: "v0", - License: "unreleased", - BitmapOps: []ext.BitmapOp{ - {Name: "Some", Func: ext.BitmapOpUnaryBitmap(Some), Reserved: []string{"p", "seed"}}, - }, -} - -// ExtensionInfo is the entry point used by the plugin code. -func ExtensionInfo(api string) (*ext.ExtensionInfo, error) { // nolint:unused,deadcode - return extInfoTemplate, nil -} - -const batchSize = 1024 - -// Some returns some of the bits from its first input bitmap. Takes seed (int) -// and p (float) values. Seed defaults to 0. -func Some(inputs []ext.Bitmap, args map[string]interface{}) ext.Bitmap { - if len(inputs) == 0 || inputs[0] == nil { - return nil - } - input := inputs[0] - min, ok := input.Min() - // no bits found? - if !ok { - return nil - } - // start at multiple of 128 not greater than min. - min &^= 127 - max := input.Max() - p, ok := args["p"].(float64) - if !ok { - return nil - } - // no bits or impossible probability range - if p <= 0 || p > 1 { - return nil - } - // every bit - if p == 1 { - return inputs[0] - } - // On failure, we default to 0. - seed, _ := args["seed"].(int64) - densityScale := uint64(256) - density := uint64(p * float64(densityScale)) - for density == 0 { - densityScale <<= 1 - density = uint64(p * float64(densityScale)) - // too small - if densityScale > (1 << 32) { - return nil - } - } - w, err := apophenia.NewWeighted(apophenia.NewSequence(seed)) - if err != nil { - return nil - } - someBits := input.New() - toAdd := make([]uint64, batchSize) - toAddN := 0 - offset := apophenia.OffsetFor(apophenia.SequenceWeighted, 0, 0, 0) - for i := min; i < max; i += 128 { - offset.Lo = i - randomBits := w.Bits(offset, density, densityScale) - bit := uint64(0) - for randomBits.Lo != 0 { - next := uint64(bits.TrailingZeros64(randomBits.Lo) + 1) - randomBits.Lo >>= next - toAdd[toAddN] = next + bit + i - toAddN++ - bit += next - } - bit = 64 - for randomBits.Hi != 0 { - next := uint64(bits.TrailingZeros64(randomBits.Hi) + 1) - randomBits.Hi >>= next - toAdd[toAddN] = next + bit + i - toAddN++ - bit += next - } - if toAddN > (batchSize - 128) { - // ignore error - _, _ = someBits.AddN(toAdd[:toAddN]...) - toAddN = 0 - } - } - if toAddN > 0 { - _, _ = someBits.AddN(toAdd[:toAddN]...) - } - return input.Intersect(someBits) -} - -func main() { - fmt.Printf("this is a plugin module only.\n") -} diff --git a/extension.go b/extension.go index d449f3f0c..1a492cce3 100644 --- a/extension.go +++ b/extension.go @@ -17,7 +17,7 @@ package pilosa import ( "fmt" - "github.com/pilosa/pilosa/v2/ext" + "github.com/molecula/ext" "github.com/pilosa/pilosa/v2/roaring" ) diff --git a/ext/extensions/distinct.go b/extensions/distinct.go similarity index 100% rename from ext/extensions/distinct.go rename to extensions/distinct.go diff --git a/ext/extensions/dummy.go b/extensions/dummy.go similarity index 100% rename from ext/extensions/dummy.go rename to extensions/dummy.go diff --git a/pql/ast.go b/pql/ast.go index 61b4b6540..738601100 100644 --- a/pql/ast.go +++ b/pql/ast.go @@ -23,7 +23,7 @@ import ( "strings" "time" - "github.com/pilosa/pilosa/v2/ext" + "github.com/molecula/ext" ) // Query represents a PQL query. diff --git a/row.go b/row.go index d9a4f9cc9..82a67cf8f 100644 --- a/row.go +++ b/row.go @@ -18,7 +18,7 @@ import ( "encoding/json" "sort" - "github.com/pilosa/pilosa/v2/ext" + "github.com/molecula/ext" "github.com/pilosa/pilosa/v2/roaring" "github.com/pkg/errors" ) diff --git a/server.go b/server.go index 645418898..44e27b1a5 100644 --- a/server.go +++ b/server.go @@ -27,9 +27,9 @@ import ( "sync" "time" - "github.com/pilosa/pilosa/v2/ext" + "github.com/molecula/ext" // extensions pulls in some extensions depending on build tags - _ "github.com/pilosa/pilosa/v2/ext/extensions" + _ "github.com/pilosa/pilosa/v2/extensions" "github.com/pilosa/pilosa/v2/logger" "github.com/pilosa/pilosa/v2/pql" "github.com/pilosa/pilosa/v2/roaring"