diff --git a/.gitlab/.gitlab-ci.yml b/.gitlab/.gitlab-ci.yml index ccd710889..0dda19409 100644 --- a/.gitlab/.gitlab-ci.yml +++ b/.gitlab/.gitlab-ci.yml @@ -221,16 +221,17 @@ deploy node for linux amd64: - echo "$AWS_SSH_PRIVATE_KEY" | ssh-add - - chmod 700 /root/.ssh - '[[ -f /.dockerenv ]] && echo -e "Host *\n\tStrictHostKeyChecking no\n\n" > ~/.ssh/config' + - apt update && apt -y install jq script: - AMI=$(aws ssm get-parameters --names /aws/service/ami-amazon-linux-latest/amzn2-ami-hvm-x86_64-ebs --query 'Parameters[0].[Value]' --output text --profile $PROFILE) - SECURITY_GROUP=$(aws ec2 describe-security-groups --filters Name=vpc-id,Values=vpc-03a4ba3d5b7c8f978 Name=group-name,Values=default --query 'SecurityGroups[*].[GroupId]' --output text --profile $PROFILE) - SUBNET_ID=$(aws ec2 describe-subnets --filters 'Name=vpc-id,Values=vpc-03a4ba3d5b7c8f978' 'Name=availability-zone,Values=us-east-2a' --query 'Subnets[0].SubnetId' --output text --profile $PROFILE) - - aws ec2 run-instances --image-id $AMI --instance-type $INSTANCE --security-group-ids $SECURITY_GROUP --subnet-id $SUBNET_ID --key-name gitlab-featurebase-dev --tag-specifications 'ResourceType=instance,Tags=[{Key=Name,Value=linux-amd64-node}]' --profile $PROFILE --user-data file://.gitlab/cloud-init.sh --iam-instance-profile Name=featurebase-dev-ssm - - sleep 120 # Need to wait for the EC2 instance to launch and run the initialization commands passed through user-data - - PUBLIC_IP=$(aws ec2 describe-instances --filters 'Name=tag:Name, Values=linux-amd64-node' 'Name=instance-state-name, Values=running' --query 'Reservations[*].Instances[*].PublicIpAddress' --output text --profile $PROFILE) - - echo $PUBLIC_IP - - INSTANCE_ID=$(aws ec2 describe-instances --filters 'Name=tag:Name, Values=linux-amd64-node' 'Name=instance-state-name, Values=running' --query 'Reservations[*].Instances[*].InstanceId' --output text --profile $PROFILE) + - aws ec2 run-instances --image-id $AMI --instance-type $INSTANCE --security-group-ids $SECURITY_GROUP --subnet-id $SUBNET_ID --key-name gitlab-featurebase-dev --tag-specifications 'ResourceType=instance,Tags=[{Key=Name,Value=linux-amd64-node}]' --profile $PROFILE --user-data file://.gitlab/cloud-init.sh --iam-instance-profile Name=featurebase-dev-ssm > config.json + - INSTANCE_ID=$(jq '.Instances | .[] |.InstanceId' config.json | tr -d '"') - echo $INSTANCE_ID > linux-amd64-instance.txt + - sleep 120 # Need to wait for the EC2 instance to launch and run the initialization commands passed through user-data + - PUBLIC_IP=$(aws ec2 describe-instances --instance-ids $INSTANCE_ID --filters 'Name=instance-state-name, Values=running' --query 'Reservations[*].Instances[*].PublicIpAddress' --output text --profile $PROFILE) + - echo $PUBLIC_IP - scp -o StrictHostKeyChecking=no -i gitlab-featurebase-dev.pem featurebase_linux_amd64 ec2-user@$PUBLIC_IP:. - scp -o StrictHostKeyChecking=no -i gitlab-featurebase-dev.pem .gitlab/featurebase.conf .gitlab/featurebase.service ec2-user@$PUBLIC_IP:. - aws ssm send-command --document-name "AWS-RunShellScript" --instance-ids $INSTANCE_ID --cli-input-json file://.gitlab/configureFeatureBase.json --profile $PROFILE --region us-east-2 diff --git a/Makefile b/Makefile index e2f659cf2..55d1332ba 100644 --- a/Makefile +++ b/Makefile @@ -104,10 +104,19 @@ build: # Create a single release build under the build directory release-build: $(MAKE) $(if $(DOCKER_BUILD),docker-)build FLAGS="-o build/featurebase-$(VERSION_ID)/featurebase" - cp NOTICE README.md LICENSE build/featurebase$(VERSION_ID) + cp NOTICE install/featurebase.conf install/featurebase*.service build/featurebase-$(VERSION_ID) tar -cvz -C build -f build/featurebase-$(VERSION_ID).tar.gz featurebase-$(VERSION_ID)/ @echo Created release build: build/featurebase-$(VERSION_ID).tar.gz +test-release-build: docker-build + mv build/featurebase-$(VERSION_ID).tar.gz install/ + cd install && docker build -t featurebase:test_installation \ + -f test_installation.Dockerfile \ + --build-arg release_tarball=featurebase-$(VERSION_ID).tar.gz . + mv install/featurebase-$(VERSION_ID).tar.gz build/ + docker run -it -v /sys/fs/cgroup:/sys/fs/cgroup:ro \ + featurebase:test_installation + # Error out if there are untracked changes in Git check-clean: ifndef SKIP_CHECK_CLEAN @@ -220,7 +229,7 @@ docker-build: vendor docker create --name featurebase-build featurebase:build mkdir -p build/featurebase-$(VERSION_ID) docker cp featurebase-build:/pilosa/build/. ./build/featurebase-$(VERSION_ID) - cp NOTICE LICENSE ./build/featurebase-$(VERSION_ID) + cp NOTICE install/featurebase.conf install/featurebase*.service ./build/featurebase-$(VERSION_ID) docker rm featurebase-build tar -cvz -C build -f build/featurebase-$(VERSION_ID).tar.gz featurebase-$(VERSION_ID)/ diff --git a/NOTICE b/NOTICE index 594272a27..cb62dfdca 100644 --- a/NOTICE +++ b/NOTICE @@ -1,23 +1,12 @@ Software license ================ -Copyright (C) 2017-2018 Pilosa Corp. All rights reserved. - -Licensed under the Apache License, Version 2.0 (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. +Copyright (C) 2017-2021 Molecula Corp. All rights reserved. Third-party software licenses ============================= -The file /pilosa/lru/lru.go contains a redistribution of lru +The file /lru/lru.go contains a redistribution of lru (github.com/golang/groupcache/lru); the license follows: Copyright 2013 Google Inc. diff --git a/install/featurebase.conf b/install/featurebase.conf new file mode 100644 index 000000000..73895ed6d --- /dev/null +++ b/install/featurebase.conf @@ -0,0 +1,373 @@ +# FEATUREBASE HOST CONFIGURATION +# +# Uncomment when/where appropriate + +# ============================================================================== +# Use advertise to specify the address advertised by the server to other nodes +# in the cluster and to clients via /status endpoint. Host defaults to IP +# address represented by bind parameter with network port. +# +# advertise = :10101 +# advertise-grpc = :20101 + + + +# "long-query-time" represents duration of time that will trigger log and stat +# message for queries longer than X time. Ex. "1m30s" 1 minute 30 seconds +# +# long-query-time = "10s" + + + +# Unique name for node in cluster. This is just a human-readable label for +# convenience and not used by any underlying logic. +# +# name = "featurebase1" + + + +# Host:Port where Featurebase server listens for HTTP requests. +# Default is localhost:10101 +# +# bind = "localhost:10101" + + + +# The address and port featurebase will listen to for all GRPC connections +# Ex. python-molecula, grafana for queries, etc. +# +# bind-grpc = "0.0.0.0:20101" + + + +# Directory to store Featurebase data files +data-dir = "/var/lib/molecula" + + +# ============================================================================== +# CORS (Cross-Origin Resource Sharing) Allowed Origins +# List of allowed origin URIs for CORS +# +# [handler] +# allowed-origins = ["https://myapp.com", "https://myapp.org"] + + + +# Path to the log file +log-path = "/var/log/molecula/featurebase.log" + + + +# Verbose - Enable verbose logging. Valid options are true or false. +# Set to true only when debugging as directed by Molecula engineers. +# +# verbose = true + + + +# Soft limit on max number of files featurebase will keep open simultaneously. +# When past this limit, featurebase will only keep files open for as long as is +# needed to write updates. +# +# max-file-count = 900000 + + + +# Maximum number of active memory maps featurebase will use for fragment files. +# Actual total usage may be slightly higher. +# Best practice is to set this to ~10% lower than your system's max map count. +# See sysctl vm.max_map_count in Linux. +# +# max-map-count = 900000 + + + +# Max Writes Per Request - Max number of mutating commands allowed per request. +# This includes Set, Clear, ClearRow, and Store +# +# max-writes-per-request = 5000 + + + +# The following option sets the maximum number of queries that are maintained +# for the /query-history endpoint. +# This parameter is per-node, and the result combines the history from all nodes. +# +# query-history-length = 100 + + + +# External database to connect to for `ExternalLookup` queries. +# lookup-db-dsn = "postgres://localhost:5432/db" + + + +# ============================================================================== +# For cluster stanza, "name" represents name for cluster. Must be same on all +# nodes in cluster. "replicas" represents number of hosts each piece of data +# should be stored on. Must be greater than or equal to 1 & less than or equal +# to number of nodes in cluster. +# [cluster] +# name = "cluster1" +# replicas = 1 + + + +# ============================================================================== +# [etcd] +# etcd is the tool Featurebase uses for node-to-node, intra-cluster +# communication. etcd is embedded in the featurebase cluster rather than +# running as a separate instance. +# It's important to configure this correctly for your network and nodes, and +# that it is consistent across all nodes. +# +# The easiest setup can be used when all nodes can reach all other nodes via a +# local subnet: +# listen-peer-address = advertise-peer-address +# = (what's in the initial-cluster-list) +# = the nodes ip address (which can be reached by every +# other node +# (localhost:10401 would not work for this, as each node can't reach that) +# +# If each node is separated by a proxy, or must be reached via url / dns, you +# will need to use a more complicated setup: +# listen-peer-address = the nodes local ip address +# (specific ip, localhost, or 0.0.0.0 for all +# local ip's) +# advertise-peer-address = the nodes ip address, reachable by all other nodes +# (This address should also be included in +# initital-cluster-list) +# in this case, you specify a different url/ip for listen-peer and +# advertise-peer. E.g. you specify 0.0.0.0 for listen, or (like in their case) +# you use a url for advertise. In each of these cases, you should set listen +# to the local ip, and you set advertise = to how each other node connects to +# this node, and you also use this same address in the initial cluster. +# The key here is that initial-cluster has to include the same node name and +# advertise-peer address as the node it's on (edited) + + + +# for additional assistance, and for help with config issues, +# see https://etcd.io/docs/v3.5/faq/ cluster-url - URL of existing cluster +# that a new node should join when adding nodes to cluster. +# +# cluster-url = "http://localhost:10401" + + + +# Address and port to bind to for client communication +# listen-client-address = "http://localhost:10401" + + + +# Address and port to bind to for peer communication +# listen-peer-address = "http://localhost:10301" + + + +# Comma-separated list of node=address pairs that makes up initial cluster when +# first started. In each pair, "node" value (left side of = ) should match +# name of node specified by "name" configuration parameter +# +# initial-cluster = "featurebase1=http://localhost:10301" + + + +# ============================================================================== +# Profile Block Rate - Block Rate is passed directly to Go's +# runtime.SetBlockProfileRate. Goroutine blocking events will be sampled at 1 +# per rate nanoseconds. A value of "1" samples every event, and 0 disables +# profiling. +# +# block-rate = 10000000 + +# Profile Mutex Fraction - Mutex Fraction is passed directly to Go's +# runtime.SetMutexProfileFraction. 1/ fraction of events will be sampled. +# +# mutex-fraction = 100 + + + +# ============================================================================== +# PostgreSQL Section +# [postgres] +# +# Endpoint Bind - Address to bind a PostgreSQL wire protocol endpoint. +# No PostgreSQL endpoint will be exposed unless a bind address is specified. +# Requires Molecula v3.0 or newer. +# +# bind = "localhost:55432" + + + +# The PostgreSQL endpoint has support for a connection limit. +# This is generally not necessary, so it is disabled by default. +# +# connection-limit = 10000 + + + +# PostgreSQL Max Startup Packet Size - By default, the postgres endpoint +# uses an 8 MiB limit on incoming PostgreSQL startup packets. This should +# typically be sufficient, but may be exceeded if a client sends an unusually +# large amount of configuration data. Oversized startup packets are typically +# caused by connecting with a different protocol, e.g. HTTP. +# +# max-startup-size = 10000000 + + + +# PostgreSQL Timeouts +# In order to detect stalled clients, the PostgreSQL endpoint has connection +# read and write timeouts. There is also a startup timeout, which is used for +# connection setup. The read timeout does not impact idle connections. Idle +# connections will only be closed by the server if TCP keepalive reports a +# break in the connection. TCP keepalives use the default configuration +# provided by the host. +# Caution: Due to a limitation of the PostgreSQL wire protocol, +# raising the write timeout may delay the shutdown of a featurebase node. +# +# startup-timeout = "20s" +# read-timeout = "20s" +# write-timeout - "20s" + + + +# Postgres Endpoint TLS - TLS configuration for the PostgreSQL endpoint is +# structured the same as the TLS configuration for Featurebase's other endpoints, +# but placed under [postgres.tls]. If TLS is configured on the postgres endpoint, +# Featurebase will reject unsecured connections. +# [postgres.tls] +# certificate = "/srv/pilosa/certs/server.crt" +# key = "/srv/pilosa/certs/server.key" +# ca-certificate = "/srv/pilosa/certs/ca.crt" +# enable-client-verification = true + + + +# ============================================================================== +# Usage Duty Cycle - Featurebase maintains a disk/memory usage cache that is +# calculated periodically in the background and accessed by the UI/usage +# endpoint. Since this disk scan can take a long and unpredictable amount of +# time, its timing behavior is specified in a relative, rather than absolute +# sense. That is, the duty cycle sets the percentage of time that is spent +# recalculating this cache. This setting affects the results received from +# the "/ui/usage" http endpoint, as well as all data file and memory usage +# values and graphs on the webui "tables" page + +# Special considerations: +# * If disk usage can be calculated quickly (less than 5 seconds), fresh +# results will be calculated when accessed +# * When disk usage takes longer to calculate, there is a minimum of one +# hour wait between cache recalculations +# Setting this value to 0 will completely disable the calculation of disk usage +# +# usage-duty-cycle = 20 + + + +# ============================================================================== +# Use [metric] stanza to define attributes for monitoring. +# [metric] +# Specify which service to use for collecting metrics. Valid options are: +# "statsd", "expvar", "prometheus", "none" +# +# service = "prometheus" + + + +# Remote host to send statsd metrics to. +# host = "localhost:8125" + + + +# The interval to send statsd metrics. +# poll-interval = "10s" + + + +# Debugging flag to enable to send diagnostic information to Featurebase +# developers. +# +# diagnostics = false + + + +# ============================================================================== +# TLS Certificate Section - Path to TLC certificate used for service HTTPS. +# Suffix should contain .crt or .pem +# +# [tls] +# certificate = "/srv/pilosa/certs/server.crt" +# TLS Certificate Key - Path to TLS certificate key for HTTPS. Suffix should +# be .key +# +# key = "/srv/pilosa/certs/server.key" + + + +# ============================================================================== +# Tracing Section +# [tracing] +# +# Jaeger sampler type. Valid options are: "const, "probabilistic", "ratelimiting", +# or "remote". Set to 'off' to disable tracing completely. +# +# sampler-type = "remote" + + +# Jaeger sampler parameter (number) +# sampler-param = 0.001 + + + +# Tracing Agent Host:Port +# agent-host-port = "localhost:6831" + + + +# ============================================================================== +# Configuration for the RBF storage format. +# [rbf] +# Maximum size for each RBF database file. +# Allocates virtual memory but does not preallocate physical disk space. +# If you get into the range where you have 16000 shards on a single node +# (across all indexes), you will need to lower this in order to not run out of +# virtual address space. +# +# max-db-size = 4294967296 + + + +# Maximum size for each RBF WAL file. +# Allocates virtual memory but does not preallocate physical disk space. +# This is the same as max-db-size, but for the write-ahead log. If you set it +# smaller, set max-wal-checkpoint-size to 1/2 of this (we will likely +# condense these options in the future). +# +# max-wal-size = 4294967296 + + + +# Minimum WAL size before WAL pages can be copied to the main database file. +# min-wal-checkpoint-size = 1048576 + + + +# Maximum WAL size before transactions are halted to copy WAL pages to the +# main database file. +# +# max-wal-checkpoint-size = 2147483648 + + +# ============================================================================== +# [storage] +# Sync all changes to the file system. +# Should not be changed in production systems unless you know what you are +# doing - Should always be on unless testing or possibly while performing a +# bulk import and you are not worried about data loss +# +# fsync = true + + +# ============================================================================== diff --git a/install/featurebase.debian.service b/install/featurebase.debian.service new file mode 100644 index 000000000..162264106 --- /dev/null +++ b/install/featurebase.debian.service @@ -0,0 +1,13 @@ +[Unit] +Description="Service for FeatureBase" +After=network.target + +[Service] +RestartSec=30 +Restart=on-failure +EnvironmentFile= +User=molecula +ExecStart=/usr/local/bin/featurebase server -c /etc/featurebase.conf + +[Install] +WantedBy=multi-user.target diff --git a/install/featurebase.redhat.service b/install/featurebase.redhat.service new file mode 100644 index 000000000..cf507d8b0 --- /dev/null +++ b/install/featurebase.redhat.service @@ -0,0 +1,12 @@ +[Unit] +Description="Service for FeatureBase" + +[Service] +RestartSec=30 +Restart=on-failure +EnvironmentFile= +User=molecula +ExecStart=/usr/local/bin/featurebase server -c /etc/featurebase.conf + +[Install] +WantedBy=multi-user.target diff --git a/install/test_installation.Dockerfile b/install/test_installation.Dockerfile new file mode 100644 index 000000000..5d9a93253 --- /dev/null +++ b/install/test_installation.Dockerfile @@ -0,0 +1,18 @@ +FROM fedora:rawhide + +RUN yum -y install systemd procps + +ARG release_tarball + +COPY $release_tarball . + +COPY test_installation.sh test_installation.sh + +RUN mkdir install && \ + tar -xf $release_tarball -C install --strip-components 1 && \ + cd install && \ + cp featurebase /usr/local/bin/featurebase && \ + cp featurebase.redhat.service /etc/systemd/system/featurebase.service && \ + cp featurebase.conf /etc/featurebase.conf + +CMD ["/bin/bash", "test_installation.sh"] diff --git a/install/test_installation.sh b/install/test_installation.sh new file mode 100644 index 000000000..4a301ea48 --- /dev/null +++ b/install/test_installation.sh @@ -0,0 +1,29 @@ +#!/bin/bash +tests_failed=0 + +# test validity of featurebase.service +systemd-analyze verify /etc/systemd/system/featurebase.service +exit_code=$? +if [ $exit_code -ne 0 ]; then + echo 'featurebase.redhat.service invalid' + tests_failed=1 +fi + +# test validity of featurebase.conf +mkdir /var/log/molecula +featurebase_bin='/usr/local/bin/featurebase' +config_file='/etc/featurebase.conf' +if ! $featurebase_bin -c $config_file holder /dev/null 2>&1; then + echo 'featurebase.conf is invalid' + tests_failed=1 +fi + +# print success if both passed +if [ $tests_failed -eq 0 ]; then + echo 'featurebase.redhat.service is valid' + echo 'featurebase.conf is valid' +fi + +# tests_failed set to 0 if none of the tests failed +# otherwise set to non-zero +exit $tests_failed diff --git a/rbf/tx.go b/rbf/tx.go index 9ecbe9a02..812d35738 100644 --- a/rbf/tx.go +++ b/rbf/tx.go @@ -56,6 +56,27 @@ type Tx struct { // behavior where an existing container has all its bits cleared // but still sticks around in the database. DeleteEmptyContainer bool + + // It is possible for a modification of the free list to cause a page to + // be allocated or deallocated, which would modify the free list. + // + // For the case where pages need to be allocated during free list + // changes, we can trivially just allocate new pages and not use the + // free list. That's the simple case... + modifyingFreelist bool + // But removals can't be deferred/not-done like that. If a free list + // change causes us to deallocate a page (such as if we're *allocating* + // a page, which causes it to be *removed* from the free list), we really + // do need to record that, but if we try to do it during the update + // process, things could go horribly wrong. So, we have a transient list + // of page numbers which have been deallocated, but it happened *during* + // the modification of the free list. The top-level modification then + // processes them on its way out, using a defer. During the processing + // of this list, we *still* have the flag set, and we make a new list + // while processing the list, so if somehow a pending add to the list + // manages to trigger a *deallocation* (which I don't think should be + // happening), we'll process that one after the current list is processed. + pendingFreelistAdds []uint32 } func (tx *Tx) DBPath() string { @@ -903,16 +924,67 @@ func (tx *Tx) walkTree(pgno, parent uint32, fn func(pgno, parent, typ uint32) er } } +// freelistCleanup handles things which we need to add to the free list, +// because they became free during the process of modifying the free list. +// It also marks us as done modifying the free list. Expected usage is that +// you set modifyingFreelist to true, then defer this. +// +// if you are modifying the free list, we can't further change the free +// list during that modification. for page allocations, we can just skip +// the free list check. for deallocations, though, we do need to mark them +// as freed at some point. so, if we're modifying the free list when +// a new freePgno happens, we stash the new pages in here, then apply +// them afterwards. so far as i know, this can actually only happen +// during an allocate, when we're removing entries from the free list, and +// the add path doesn't ever trigger it. so, when we remove entries from +// the free list, it's possible that doing so frees up pages that were +// part of the free list, and we then add them. but we don't have to worry +// about that removing things from the free list, because the add logic +// already just uses new pages rather than trying to use the free list +// when it knows the free list is involved. +func (tx *Tx) freelistCleanup(outErr *error) { + defer func() { + // no matter what, we're done with this after this, but we still + // want it set *while* we do this so nothing we do will have side + // effects that collide with what we're doing. + tx.modifyingFreelist = false + }() + if len(tx.pendingFreelistAdds) == 0 { + return + } + c := Cursor{tx: tx} + c.stack.elems[0] = stackElem{pgno: readMetaFreelistPageNo(tx.meta[:])} + for len(tx.pendingFreelistAdds) > 0 { + var pass []uint32 + pass, tx.pendingFreelistAdds = tx.pendingFreelistAdds, nil + for _, pgno := range pass { + if changed, err := c.Add(uint64(pgno)); err != nil { + if outErr != nil && *outErr == nil { + *outErr = err + } + return + } else if !changed { + PanicOn(fmt.Sprintf("rbf.Tx.freePgno(): double free: %d", tx.pendingFreelistAdds)) + } + } + } +} + // allocatePgno returns a page number for a new available page. This page may be // pulled from the free list or, if no free pages are available, it will be // created by extending the file size. -func (tx *Tx) allocatePgno() (uint32, error) { +func (tx *Tx) allocatePgno() (_ uint32, outErr error) { + if tx.modifyingFreelist { + return tx.allocateNewPgno(), nil + } // Attempt to find page in freelist. pgno, err := tx.nextFreelistPageNo() if err != nil { return 0, err } else if pgno != 0 { + tx.modifyingFreelist = true + defer tx.freelistCleanup(&outErr) c := Cursor{tx: tx} c.stack.elems[0] = stackElem{pgno: readMetaFreelistPageNo(tx.meta[:])} if changed, err := c.Remove(uint64(pgno)); err != nil { @@ -922,11 +994,16 @@ func (tx *Tx) allocatePgno() (uint32, error) { } return pgno, nil } + // no freelist pages, fall back + return tx.allocateNewPgno(), nil +} +// allocateNewPgno requests a new page unconditionally, ignoring the free list. +func (tx *Tx) allocateNewPgno() uint32 { // Increment the total page count by one and return the last page. - pgno = readMetaPageN(tx.meta[:]) + pgno := readMetaPageN(tx.meta[:]) writeMetaPageN(tx.meta[:], pgno+1) - return pgno, nil + return pgno } func (tx *Tx) nextFreelistPageNo() (uint32, error) { @@ -952,10 +1029,16 @@ func (tx *Tx) nextFreelistPageNo() (uint32, error) { } // deallocate releases a page number to the freelist. -func (tx *Tx) freePgno(pgno uint32) error { +func (tx *Tx) freePgno(pgno uint32) (outErr error) { + if tx.modifyingFreelist { + tx.pendingFreelistAdds = append(tx.pendingFreelistAdds, pgno) + return nil + } c := Cursor{tx: tx} c.stack.elems[0] = stackElem{pgno: readMetaFreelistPageNo(tx.meta[:])} + tx.modifyingFreelist = true + defer tx.freelistCleanup(&outErr) if changed, err := c.Add(uint64(pgno)); err != nil { return err } else if !changed { @@ -979,7 +1062,7 @@ func (tx *Tx) deallocateTree(pgno uint32) error { return err } } - return nil + return tx.freePgno(pgno) case PageTypeLeaf: return tx.freePgno(pgno) diff --git a/rbf/tx_test.go b/rbf/tx_test.go index f7cd78d6a..e4b68f4b6 100644 --- a/rbf/tx_test.go +++ b/rbf/tx_test.go @@ -22,6 +22,7 @@ import ( "time" "github.com/molecula/featurebase/v2/rbf" + "github.com/molecula/featurebase/v2/roaring" ) func TestTx_CommitRollback(t *testing.T) { @@ -219,6 +220,95 @@ func TestTx_DeleteBitmap(t *testing.T) { } } +// deallocateTree had a bug which caused a page to be marked neither free +// nor in-use. +func TestTx_DeallocateTree(t *testing.T) { + db := MustOpenDB(t) + defer MustCloseDB(t, db) + tx := MustBegin(t, db, true) + defer tx.Rollback() + var err error + + // Create bitmap & add value. + if err = tx.CreateBitmap("x"); err != nil { + t.Fatal(err) + } + const N = 315 + slots := make([]uint64, N) + for i := range slots { + slots[i] = uint64(i) << 20 + } + if _, err = tx.Add("x", slots...); err != nil { + t.Fatal(err) + } + if err = tx.Check(); err != nil { + t.Fatalf("check: %v", err) + } + if err = tx.DeleteBitmap("x"); err != nil { + t.Fatal(err) + } + if ok, err := tx.Contains("x", 0); err != nil { + t.Fatal(err) + } else if ok { + t.Fatal("expected no value in recreated bitmap") + } + if err = tx.Check(); err != nil { + t.Fatalf("check: %v", err) + } +} + +func TestTx_RecreateBitmap(t *testing.T) { + db := MustOpenDB(t) + defer MustCloseDB(t, db) + tx := MustBegin(t, db, true) + defer tx.Rollback() + + // Create bitmap & add value. + if err := tx.CreateBitmap("x"); err != nil { + t.Fatal(err) + } + const N = 825000 + slots := make([]uint64, N) + for i := range slots { + slots[i] = uint64(i) << 20 + } + if _, err := tx.Add("x", slots...); err != nil { + t.Fatal(err) + } + err := tx.Commit() + if err != nil { + t.Fatal(err) + } + tx = MustBegin(t, db, true) + defer tx.Rollback() + // Delete bitmap, verifying that it's gone. + if err := tx.DeleteBitmap("x"); err != nil { + t.Fatal(err) + } else { + if ok, err := tx.Contains("x", 0); err != nil { + t.Fatal(err) + } else if ok { + t.Fatal("expected no value in recreated bitmap") + } + } + err = tx.Commit() + if err != nil { + t.Fatal(err) + } + tx = MustBegin(t, db, true) + defer tx.Rollback() + if err := tx.CreateBitmap("x"); err != nil { + t.Fatal(err) + } + if _, err := tx.Add("x", slots...); err != nil { + t.Fatal(err) + } + err = tx.Commit() + if err != nil { + t.Fatal(err) + } +} + func TestTx_RenameBitmap(t *testing.T) { db := MustOpenDB(t) defer MustCloseDB(t, db) @@ -279,6 +369,83 @@ func TestTx_Add_Quick(t *testing.T) { }) } +func TestTx_DeallocateToFreeList(t *testing.T) { + db := MustOpenDB(t) + defer MustCloseDB(t, db) + tx := MustBegin(t, db, true) + defer tx.Rollback() + var err error + + // Create bitmap & add value. + if err = tx.CreateBitmap("x"); err != nil { + t.Fatal(err) + } + if err = tx.CreateBitmap("y"); err != nil { + t.Fatal(err) + } + const N = 12274831 + slots := make([]uint64, N) + for i := range slots { + slots[i] = uint64(i) << 10 + } + bm := roaring.NewBitmap(slots...) + if _, err = tx.AddRoaring("x", bm); err != nil { + t.Fatal(err) + } + if err = tx.Check(); err != nil { + t.Fatal(err) + } + for i := 0; i < 500; i++ { + if _, err := tx.Add("y", uint64(i)<<16); err != nil { + t.Fatal(err) + } + } + if err = tx.Check(); err != nil { + t.Fatal(err) + } + if err := tx.DeleteBitmap("y"); err != nil { + t.Fatal(err) + } + if err = tx.Check(); err != nil { + t.Fatal(err) + } + if err = tx.Commit(); err != nil { + t.Fatal(err) + } + tx = MustBegin(t, db, true) + defer tx.Rollback() + // Delete bitmap, verifying that it's gone. + if err := tx.DeleteBitmap("x"); err != nil { + t.Fatal(err) + } else { + if ok, err := tx.Contains("x", 0); err != nil { + t.Fatal(err) + } else if ok { + t.Fatal("expected no value in recreated bitmap") + } + } + if err = tx.Check(); err != nil { + t.Fatal(err) + } + if err = tx.Commit(); err != nil { + t.Fatal(err) + } + tx = MustBegin(t, db, true) + defer tx.Rollback() + if err := tx.CreateBitmap("x"); err != nil { + t.Fatal(err) + } + if _, err := tx.AddRoaring("x", bm); err != nil { + t.Fatal(err) + } + if err = tx.Check(); err != nil { + t.Fatal(err) + } + if err = tx.Commit(); err != nil { + t.Fatal(err) + } +} + func TestTx_AddRemove_Quick(t *testing.T) { if testing.Short() { t.Skip("-short enabled, skipping")