Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Fix the go build and use CgoArrowAllocator to prevent incorrect garbage collection #2919

Merged
merged 39 commits into from
Jul 8, 2022
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions .github/workflows/build_wheels.yml
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,18 @@ jobs:
registry-url: 'https://registry.npmjs.org'
- name: Build UI
run: make build-ui
- name: Install apache-arrow on ubuntu
if: matrix.os == 'ubuntu-latest'
run: |
sudo apt update
sudo apt install -y -V ca-certificates lsb-release wget
wget https://apache.jfrog.io/artifactory/arrow/$(lsb_release --id --short | tr 'A-Z' 'a-z')/apache-arrow-apt-source-latest-$(lsb_release --codename --short).deb
sudo apt install -y -V ./apache-arrow-apt-source-latest-$(lsb_release --codename --short).deb
sudo apt update
sudo apt install -y -V libarrow-dev
- name: Install apache-arrow on macos
if: matrix.os == 'macOS-latest'
run: brew install apache-arrow
- name: Build wheels
uses: pypa/cibuildwheel@v2.7.0
env:
Expand Down
18 changes: 17 additions & 1 deletion .github/workflows/linter.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,14 @@ jobs:
${{ runner.os }}-${{ steps.setup-python.outputs.python-version }}-pip-
- name: Install pip-tools
run: pip install pip-tools
- name: Install apache-arrow on ubuntu
run: |
sudo apt update
sudo apt install -y -V ca-certificates lsb-release wget
wget https://apache.jfrog.io/artifactory/arrow/$(lsb_release --id --short | tr 'A-Z' 'a-z')/apache-arrow-apt-source-latest-$(lsb_release --codename --short).deb
sudo apt install -y -V ./apache-arrow-apt-source-latest-$(lsb_release --codename --short).deb
sudo apt update
sudo apt install -y -V libarrow-dev
- name: Install dependencies
run: |
make compile-protos-go
Expand All @@ -63,7 +71,15 @@ jobs:
- name: Upgrade pip version
run: |
pip install --upgrade "pip>=21.3.1,<22.1"
- name: Install apache-arrow on ubuntu
run: |
sudo apt update
sudo apt install -y -V ca-certificates lsb-release wget
wget https://apache.jfrog.io/artifactory/arrow/$(lsb_release --id --short | tr 'A-Z' 'a-z')/apache-arrow-apt-source-latest-$(lsb_release --codename --short).deb
sudo apt install -y -V ./apache-arrow-apt-source-latest-$(lsb_release --codename --short).deb
sudo apt update
sudo apt install -y -V libarrow-dev
- name: Install dependencies
run: make install-go-proto-dependencies
run: make compile-go-lib
- name: Lint go
run: make lint-go
12 changes: 12 additions & 0 deletions .github/workflows/master_only.yml
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,18 @@ jobs:
${{ runner.os }}-${{ steps.setup-python.outputs.python-version }}-pip-
- name: Install pip-tools
run: pip install pip-tools
- name: Install apache-arrow on ubuntu
if: matrix.os == 'ubuntu-latest'
run: |
sudo apt update
sudo apt install -y -V ca-certificates lsb-release wget
wget https://apache.jfrog.io/artifactory/arrow/$(lsb_release --id --short | tr 'A-Z' 'a-z')/apache-arrow-apt-source-latest-$(lsb_release --codename --short).deb
sudo apt install -y -V ./apache-arrow-apt-source-latest-$(lsb_release --codename --short).deb
sudo apt update
sudo apt install -y -V libarrow-dev
- name: Install apache-arrow on macos
if: matrix.os == 'macOS-latest'
run: brew install apache-arrow
- name: Install dependencies
run: make install-python-ci-dependencies
- name: Setup Redis Cluster
Expand Down
12 changes: 12 additions & 0 deletions .github/workflows/pr_integration_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,18 @@ jobs:
${{ runner.os }}-${{ steps.setup-python.outputs.python-version }}-pip-
- name: Install pip-tools
run: pip install pip-tools
- name: Install apache-arrow on ubuntu
if: matrix.os == 'ubuntu-latest'
run: |
sudo apt update
sudo apt install -y -V ca-certificates lsb-release wget
wget https://apache.jfrog.io/artifactory/arrow/$(lsb_release --id --short | tr 'A-Z' 'a-z')/apache-arrow-apt-source-latest-$(lsb_release --codename --short).deb
sudo apt install -y -V ./apache-arrow-apt-source-latest-$(lsb_release --codename --short).deb
sudo apt update
sudo apt install -y -V libarrow-dev
- name: Install apache-arrow on macos
if: matrix.os == 'macOS-latest'
run: brew install apache-arrow
- name: Install dependencies
run: make install-python-ci-dependencies
- name: Setup Redis Cluster
Expand Down
28 changes: 24 additions & 4 deletions .github/workflows/unit_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,18 @@ jobs:
${{ runner.os }}-${{ steps.setup-python.outputs.python-version }}-pip-
- name: Install pip-tools
run: pip install pip-tools
- name: Install apache-arrow on ubuntu
if: matrix.os == 'ubuntu-latest'
run: |
sudo apt update
sudo apt install -y -V ca-certificates lsb-release wget
wget https://apache.jfrog.io/artifactory/arrow/$(lsb_release --id --short | tr 'A-Z' 'a-z')/apache-arrow-apt-source-latest-$(lsb_release --codename --short).deb
sudo apt install -y -V ./apache-arrow-apt-source-latest-$(lsb_release --codename --short).deb
sudo apt update
sudo apt install -y -V libarrow-dev
- name: Install apache-arrow on macos
if: matrix.os == 'macOS-latest'
run: brew install apache-arrow
- name: Install dependencies
run: make install-python-ci-dependencies
- name: Test Python
Expand All @@ -77,6 +89,8 @@ jobs:

unit-test-go:
runs-on: ubuntu-latest
env:
PYTHON: "3.8"
steps:
- uses: actions/checkout@v2
- name: Setup Python
Expand All @@ -92,9 +106,15 @@ jobs:
uses: actions/setup-go@v2
with:
go-version: 1.18.0
- name: Install dependencies
run: make install-go-proto-dependencies
- name: Compile protos
run: make compile-protos-go
- name: Install pip-tools
run: pip install pip-tools
- name: Install apache-arrow on ubuntu
run: |
sudo apt update
sudo apt install -y -V ca-certificates lsb-release wget
wget https://apache.jfrog.io/artifactory/arrow/$(lsb_release --id --short | tr 'A-Z' 'a-z')/apache-arrow-apt-source-latest-$(lsb_release --codename --short).deb
sudo apt install -y -V ./apache-arrow-apt-source-latest-$(lsb_release --codename --short).deb
sudo apt update
sudo apt install -y -V libarrow-dev
- name: Test
run: make test-go
5 changes: 2 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ install-go-proto-dependencies:
install-go-ci-dependencies:
# TODO: currently gopy installation doesn't work w/o explicit go get in the next line
# TODO: there should be a better way to install gopy
go get github.com/go-python/gopy@v0.4.0
go get github.com/go-python/gopy@v0.4.4
go install golang.org/x/tools/cmd/goimports
# The `go get` command on the previous lines download the lib along with replacing the dep to `feast-dev/gopy`
# but the following command is needed to install it for some reason.
Expand All @@ -189,8 +189,7 @@ compile-go-lib: install-go-proto-dependencies install-go-ci-dependencies
COMPILE_GO=True python setup.py build_ext --inplace

# Needs feast package to setup the feature store
test-go: compile-protos-go
pip install -e ".[ci]"
test-go: install-python-ci-dependencies compile-go-lib
go test ./...

format-go:
Expand Down
15 changes: 13 additions & 2 deletions docs/reference/feature-servers/go-feature-retrieval.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,19 @@ However, some additional dependencies are required for Go <-> Python interoperab
```
pip install feast[go]
```
You will also have to install the apache-arrow c++ libraries.
For macos, run `brew install apache-arrow`.
For linux users, you have to install `libarrow-dev`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain why this is the case?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added

```
sudo apt update
sudo apt install -y -V ca-certificates lsb-release wget
wget https://apache.jfrog.io/artifactory/arrow/$(lsb_release --id --short | tr 'A-Z' 'a-z')/apache-arrow-apt-source-latest-$(lsb_release --codename --short).deb
sudo apt install -y -V ./apache-arrow-apt-source-latest-$(lsb_release --codename --short).deb
sudo apt update
sudo apt install -y -V libarrow-dev # For C++
```

For developers, if you want to build from source, run `make compile-go-lib` to build and compile the go server.
For developers, if you want to build from source, run `make compile-go-lib` to build and compile the go server. In order to build the go binaries, you will need to install the `apache-arrow` c++ libraries.

## Usage

Expand Down Expand Up @@ -63,7 +74,7 @@ feature_server:
emit_timeout_micro_secs: 10000
queue_capacity: 10000
```
All these parameters are optional.
All these parameters are optional.

## Future/Current Work

Expand Down
8 changes: 4 additions & 4 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,10 @@ module github.com/feast-dev/feast

go 1.17

replace github.com/go-python/gopy v0.4.0 => github.com/feast-dev/gopy v0.4.1-0.20220429180328-4257ac71a4d0

require (
github.com/apache/arrow/go/v8 v8.0.0
github.com/ghodss/yaml v1.0.0
github.com/go-python/gopy v0.4.0
github.com/go-python/gopy v0.4.4
github.com/go-redis/redis/v8 v8.11.4
github.com/golang/protobuf v1.5.2
github.com/google/uuid v1.3.0
Expand All @@ -28,6 +26,8 @@ require (
github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f // indirect
github.com/goccy/go-json v0.9.6 // indirect
github.com/golang/snappy v0.0.4 // indirect
github.com/gonuts/commander v0.1.0 // indirect
github.com/gonuts/flag v0.1.0 // indirect
github.com/google/flatbuffers v2.0.6+incompatible // indirect
github.com/klauspost/asmfmt v1.3.2 // indirect
github.com/klauspost/compress v1.15.1 // indirect
Expand All @@ -38,7 +38,7 @@ require (
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/zeebo/xxh3 v1.0.2 // indirect
golang.org/x/exp v0.0.0-20220407100705-7b9b53b0aca4 // indirect
golang.org/x/mod v0.6.0-dev.0.20220106191415-9b9b3d81d5e3 // indirect
golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4 // indirect
golang.org/x/net v0.0.0-20220407224826-aac1ed45d8e3 // indirect
golang.org/x/sys v0.0.0-20220412211240-33da011f77ad // indirect
golang.org/x/text v0.3.7 // indirect
Expand Down
9 changes: 6 additions & 3 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,6 @@ github.com/envoyproxy/go-control-plane v0.9.10-0.20210907150352-cf90f659a021/go.
github.com/envoyproxy/go-control-plane v0.10.2-0.20220325020618-49ff273808a1/go.mod h1:KJwIaB5Mv44NWtYuAOFCVOjcI94vtpEz2JU/D2v6IjE=
github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c=
github.com/fatih/color v1.7.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5KwzbycvMj4=
github.com/feast-dev/gopy v0.4.1-0.20220429180328-4257ac71a4d0 h1:Go714ObVP1O+a6qK7haXVL28QNm6WMD8bwnN9EA8PlM=
github.com/feast-dev/gopy v0.4.1-0.20220429180328-4257ac71a4d0/go.mod h1:ZO6vpitQ61NVoQP/2yOubPS6ET5pP3CAWCiMYn5eqCc=
github.com/fogleman/gg v1.2.1-0.20190220221249-0403632d5b90/go.mod h1:R/bRT+9gY/C5z7JzPU0zXsXHKM4/ayA+zqcVNZzPa1k=
github.com/fogleman/gg v1.3.0/go.mod h1:R/bRT+9gY/C5z7JzPU0zXsXHKM4/ayA+zqcVNZzPa1k=
github.com/franela/goblin v0.0.0-20200105215937-c9ffbefa60db/go.mod h1:7dvUGVsVBjqR7JHJk0brhHOZYGmfBYOrK0ZhYMEtBr4=
Expand All @@ -112,6 +110,8 @@ github.com/go-logfmt/logfmt v0.3.0/go.mod h1:Qt1PoO58o5twSAckw1HlFXLmHsOX5/0LbT9
github.com/go-logfmt/logfmt v0.4.0/go.mod h1:3RMwSq7FuexP4Kalkev3ejPJsZTpXXBr9+V4qmtdjCk=
github.com/go-logfmt/logfmt v0.5.0/go.mod h1:wCYkCAKZfumFQihp8CzCvQ3paCTfi41vtzG1KdI/P7A=
github.com/go-logr/logr v0.4.0/go.mod h1:z6/tIYblkpsD+a4lm/fGIIU9mZ+XfAiaFtq7xTgseGU=
github.com/go-python/gopy v0.4.4 h1:3LTsrfVcmg2VEM6wU+eh4d9EZn5H2iogObXjiQHrF8Q=
github.com/go-python/gopy v0.4.4/go.mod h1:tlA/KcD7rM8B+NQJR4SASwiinfKY0aiMFanHszR8BZA=
github.com/go-redis/redis/v8 v8.11.4 h1:kHoYkfZP6+pe04aFTnhDH6GDROa5yJdHJVNxV3F46Tg=
github.com/go-redis/redis/v8 v8.11.4/go.mod h1:2Z2wHZXdQpCDXEGzqMockDpNyYvi2l4Pxt6RJr792+w=
github.com/go-sql-driver/mysql v1.4.0/go.mod h1:zAC/RDZ24gD3HViQzih4MyKcchzm+sOG5ZlKdlhCg5w=
Expand Down Expand Up @@ -147,7 +147,9 @@ github.com/golang/protobuf v1.5.2/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiu
github.com/golang/snappy v0.0.0-20180518054509-2e65f85255db/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q=
github.com/golang/snappy v0.0.4 h1:yAGX7huGHXlcLOEtBnF4w7FQwA26wojNCwOYAEhLjQM=
github.com/golang/snappy v0.0.4/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q=
github.com/gonuts/commander v0.1.0 h1:EcDTiVw9oAVORFjQOEOuHQqcl6OXMyTgELocTq6zJ0I=
github.com/gonuts/commander v0.1.0/go.mod h1:qkb5mSlcWodYgo7vs8ulLnXhfinhZsZcm6+H/z1JjgY=
github.com/gonuts/flag v0.1.0 h1:fqMv/MZ+oNGu0i9gp0/IQ/ZaPIDoAZBOBaJoV7viCWM=
github.com/gonuts/flag v0.1.0/go.mod h1:ZTmTGtrSPejTo/SRNhCqwLTmiAgyBdCkLYhHrAoBdz4=
github.com/google/btree v0.0.0-20180813153112-4030bb1f1f0c/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ=
github.com/google/btree v1.0.0/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ=
Expand Down Expand Up @@ -439,8 +441,9 @@ golang.org/x/mod v0.5.1/go.mod h1:5OXOZSfqPIIbmVBIIKWRFfZjPR0E5r58TLhUjH0a2Ro=
golang.org/x/mod v0.5.1/go.mod h1:5OXOZSfqPIIbmVBIIKWRFfZjPR0E5r58TLhUjH0a2Ro=
golang.org/x/mod v0.6.0-dev.0.20211013180041-c96bc1413d57/go.mod h1:3p9vT2HGsQu2K1YbXdKPJLVgG5VJdoTa1poYQBtP1AY=
golang.org/x/mod v0.6.0-dev.0.20211013180041-c96bc1413d57/go.mod h1:3p9vT2HGsQu2K1YbXdKPJLVgG5VJdoTa1poYQBtP1AY=
golang.org/x/mod v0.6.0-dev.0.20220106191415-9b9b3d81d5e3 h1:kQgndtyPBW/JIYERgdxfwMYh3AVStj88WQTlNDi2a+o=
golang.org/x/mod v0.6.0-dev.0.20220106191415-9b9b3d81d5e3/go.mod h1:3p9vT2HGsQu2K1YbXdKPJLVgG5VJdoTa1poYQBtP1AY=
golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4 h1:6zppjxzCulZykYSLyVDYbneBfbaBIQPYMevg0bEwv2s=
golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4=
golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
golang.org/x/net v0.0.0-20180906233101-161cd47e91fd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
Expand Down
2 changes: 1 addition & 1 deletion go/embedded/online_features.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ func (s *OnlineFeatureService) GetOnlineFeatures(

outputFields := make([]arrow.Field, 0)
outputColumns := make([]arrow.Array, 0)
pool := memory.NewGoAllocator()
pool := memory.NewCgoArrowAllocator()
for _, featureVector := range resp {
outputFields = append(outputFields,
arrow.Field{
Expand Down
5 changes: 4 additions & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,7 @@ def build_extension(self, ext: Extension):
destination = os.path.dirname(os.path.abspath(self.get_ext_fullpath(ext.name)))
subprocess.check_call(["go", "install", "golang.org/x/tools/cmd/goimports"],
env={"PATH": bin_path, **go_env})
subprocess.check_call(["go", "get", "github.com/go-python/gopy@v0.4.0"],
subprocess.check_call(["go", "get", "github.com/go-python/gopy@v0.4.4"],
env={"PATH": bin_path, **go_env})
subprocess.check_call(["go", "install", "github.com/go-python/gopy"],
env={"PATH": bin_path, **go_env})
Expand All @@ -442,6 +442,9 @@ def build_extension(self, ext: Extension):
destination,
"-vm",
sys.executable,
"--build-tags",
'cgo ccalloc',
"--dynamic-link=True",
"-no-make",
*ext.sources,
],
Expand Down