Skip to content

Commit 130746e

Browse files
kevjumbakevjumba
authored
fix: Fix the go build and use CgoArrowAllocator to prevent incorrect garbage collection (#2919)
* Temp fix Signed-off-by: Kevin Zhang <[email protected]> * Temp fix Signed-off-by: Kevin Zhang <[email protected]> * Fix Signed-off-by: Kevin Zhang <[email protected]> * Fix Signed-off-by: Kevin Zhang <[email protected]> * add dynamic linking flags Signed-off-by: Kevin Zhang <[email protected]> * Update gitignore Signed-off-by: Kevin Zhang <[email protected]> * Fix Signed-off-by: Kevin Zhang <[email protected]> * Fix Signed-off-by: Kevin Zhang <[email protected]> * Fix workflows Signed-off-by: Kevin Zhang <[email protected]> * Fix Signed-off-by: Kevin Zhang <[email protected]> * Fix Signed-off-by: Kevin Zhang <[email protected]> * Fix Signed-off-by: Kevin Zhang <[email protected]> * Fix Signed-off-by: Kevin Zhang <[email protected]> * Fix Signed-off-by: Kevin Zhang <[email protected]> * Fix Signed-off-by: Kevin Zhang <[email protected]> * Fix Signed-off-by: Kevin Zhang <[email protected]> * Fix Signed-off-by: Kevin Zhang <[email protected]> * Fix Signed-off-by: Kevin Zhang <[email protected]> * Fix Signed-off-by: Kevin Zhang <[email protected]> * Fix Signed-off-by: Kevin Zhang <[email protected]> * Fix Signed-off-by: Kevin Zhang <[email protected]> * Fix Signed-off-by: Kevin Zhang <[email protected]> * Fix Signed-off-by: Kevin Zhang <[email protected]> * Fix Signed-off-by: Kevin Zhang <[email protected]> * Fix Signed-off-by: Kevin Zhang <[email protected]> * Fix Signed-off-by: Kevin Zhang <[email protected]> * Fix Signed-off-by: Kevin Zhang <[email protected]> * Fix Signed-off-by: Kevin Zhang <[email protected]> * Fix Signed-off-by: Kevin Zhang <[email protected]> * Fix Signed-off-by: Kevin Zhang <[email protected]> * fix Signed-off-by: Kevin Zhang <[email protected]> * Fix Signed-off-by: Kevin Zhang <[email protected]> * Fix Signed-off-by: Kevin Zhang <[email protected]> * Fix Signed-off-by: Kevin Zhang <[email protected]> * Fix Signed-off-by: Kevin Zhang <[email protected]> * Fix Signed-off-by: Kevin Zhang <[email protected]> * Fix Signed-off-by: Kevin Zhang <[email protected]> * fix Signed-off-by: Kevin Zhang <[email protected]> * Fix Signed-off-by: Kevin Zhang <[email protected]>
1 parent 9ae22a1 commit 130746e

File tree

12 files changed

+114
-27
lines changed

12 files changed

+114
-27
lines changed

‎.github/workflows/build_wheels.yml‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,18 @@ jobs:
6262
registry-url: 'https://registry.npmjs.org'
6363
- name: Build UI
6464
run: make build-ui
65+
- name: Install apache-arrow on ubuntu
66+
if: matrix.os == 'ubuntu-latest'
67+
run: |
68+
sudo apt update
69+
sudo apt install -y -V ca-certificates lsb-release wget
70+
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
71+
sudo apt install -y -V ./apache-arrow-apt-source-latest-$(lsb_release --codename --short).deb
72+
sudo apt update
73+
sudo apt install -y -V libarrow-dev
74+
- name: Install apache-arrow on macos
75+
if: matrix.os == 'macOS-latest'
76+
run: brew install apache-arrow
6577
- name: Build wheels
6678
uses: pypa/[email protected]
6779
env:

‎.github/workflows/linter.yml‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,14 @@ jobs:
3939
${{ runner.os }}-${{ steps.setup-python.outputs.python-version }}-pip-
4040
- name: Install pip-tools
4141
run: pip install pip-tools
42+
- name: Install apache-arrow on ubuntu
43+
run: |
44+
sudo apt update
45+
sudo apt install -y -V ca-certificates lsb-release wget
46+
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
47+
sudo apt install -y -V ./apache-arrow-apt-source-latest-$(lsb_release --codename --short).deb
48+
sudo apt update
49+
sudo apt install -y -V libarrow-dev
4250
- name: Install dependencies
4351
run: |
4452
make compile-protos-go
@@ -63,7 +71,13 @@ jobs:
6371
- name: Upgrade pip version
6472
run: |
6573
pip install --upgrade "pip>=21.3.1,<22.1"
66-
- name: Install dependencies
67-
run: make install-go-proto-dependencies
74+
- name: Install apache-arrow on ubuntu
75+
run: |
76+
sudo apt update
77+
sudo apt install -y -V ca-certificates lsb-release wget
78+
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
79+
sudo apt install -y -V ./apache-arrow-apt-source-latest-$(lsb_release --codename --short).deb
80+
sudo apt update
81+
sudo apt install -y -V libarrow-dev
6882
- name: Lint go
6983
run: make lint-go

‎.github/workflows/master_only.yml‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,18 @@ jobs:
127127
${{ runner.os }}-${{ steps.setup-python.outputs.python-version }}-pip-
128128
- name: Install pip-tools
129129
run: pip install pip-tools
130+
- name: Install apache-arrow on ubuntu
131+
if: matrix.os == 'ubuntu-latest'
132+
run: |
133+
sudo apt update
134+
sudo apt install -y -V ca-certificates lsb-release wget
135+
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
136+
sudo apt install -y -V ./apache-arrow-apt-source-latest-$(lsb_release --codename --short).deb
137+
sudo apt update
138+
sudo apt install -y -V libarrow-dev
139+
- name: Install apache-arrow on macos
140+
if: matrix.os == 'macOS-latest'
141+
run: brew install apache-arrow
130142
- name: Install dependencies
131143
run: make install-python-ci-dependencies
132144
- name: Setup Redis Cluster

‎.github/workflows/pr_integration_tests.yml‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,18 @@ jobs:
153153
${{ runner.os }}-${{ steps.setup-python.outputs.python-version }}-pip-
154154
- name: Install pip-tools
155155
run: pip install pip-tools
156+
- name: Install apache-arrow on ubuntu
157+
if: matrix.os == 'ubuntu-latest'
158+
run: |
159+
sudo apt update
160+
sudo apt install -y -V ca-certificates lsb-release wget
161+
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
162+
sudo apt install -y -V ./apache-arrow-apt-source-latest-$(lsb_release --codename --short).deb
163+
sudo apt update
164+
sudo apt install -y -V libarrow-dev
165+
- name: Install apache-arrow on macos
166+
if: matrix.os == 'macOS-latest'
167+
run: brew install apache-arrow
156168
- name: Install dependencies
157169
run: make install-python-ci-dependencies
158170
- name: Setup Redis Cluster

‎.github/workflows/unit_tests.yml‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,18 @@ jobs:
5454
${{ runner.os }}-${{ steps.setup-python.outputs.python-version }}-pip-
5555
- name: Install pip-tools
5656
run: pip install pip-tools
57+
- name: Install apache-arrow on ubuntu
58+
if: matrix.os == 'ubuntu-latest'
59+
run: |
60+
sudo apt update
61+
sudo apt install -y -V ca-certificates lsb-release wget
62+
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
63+
sudo apt install -y -V ./apache-arrow-apt-source-latest-$(lsb_release --codename --short).deb
64+
sudo apt update
65+
sudo apt install -y -V libarrow-dev
66+
- name: Install apache-arrow on macos
67+
if: matrix.os == 'macOS-latest'
68+
run: brew install apache-arrow
5769
- name: Install dependencies
5870
run: make install-python-ci-dependencies
5971
- name: Test Python
@@ -92,9 +104,13 @@ jobs:
92104
uses: actions/setup-go@v2
93105
with:
94106
go-version: 1.18.0
95-
- name: Install dependencies
96-
run: make install-go-proto-dependencies
97-
- name: Compile protos
98-
run: make compile-protos-go
107+
- name: Install apache-arrow on ubuntu
108+
run: |
109+
sudo apt update
110+
sudo apt install -y -V ca-certificates lsb-release wget
111+
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
112+
sudo apt install -y -V ./apache-arrow-apt-source-latest-$(lsb_release --codename --short).deb
113+
sudo apt update
114+
sudo apt install -y -V libarrow-dev
99115
- name: Test
100116
run: make test-go

‎Makefile‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -172,12 +172,12 @@ install-go-proto-dependencies:
172172
install-go-ci-dependencies:
173173
# TODO: currently gopy installation doesn't work w/o explicit go get in the next line
174174
# TODO: there should be a better way to install gopy
175-
go get github.com/go-python/[email protected].0
175+
go get github.com/go-python/[email protected].4
176176
go install golang.org/x/tools/cmd/goimports
177177
# The `go get` command on the previous lines download the lib along with replacing the dep to `feast-dev/gopy`
178178
# but the following command is needed to install it for some reason.
179179
go install github.com/go-python/gopy
180-
python -m pip install pybindgen==0.22.0
180+
python -m pip install pybindgen==0.22.0 protobuf==3.20.1
181181

182182
install-protoc-dependencies:
183183
pip install grpcio-tools==1.47.0 mypy-protobuf==3.1.0
@@ -186,18 +186,21 @@ compile-protos-go: install-go-proto-dependencies install-protoc-dependencies
186186
python setup.py build_go_protos
187187

188188
compile-go-lib: install-go-proto-dependencies install-go-ci-dependencies
189-
COMPILE_GO=True python setup.py build_ext --inplace
189+
CGO_LDFLAGS_ALLOW=".*" COMPILE_GO=True python setup.py build_ext --inplace
190190

191-
# Needs feast package to setup the feature store
192-
test-go: compile-protos-go
191+
install-feast-ci-locally:
193192
pip install -e ".[ci]"
194-
go test ./...
193+
194+
# Needs feast package to setup the feature store
195+
# CGO flag is due to this issue: https://github.com/golang/go/wiki/InvalidFlag
196+
test-go: compile-protos-go compile-go-lib install-feast-ci-locally
197+
CGO_LDFLAGS_ALLOW=".*" go test -tags cgo,ccalloc ./...
195198

196199
format-go:
197200
gofmt -s -w go/
198201

199-
lint-go: compile-protos-go
200-
go vet ./go/internal/feast ./go/embedded
202+
lint-go: compile-protos-go compile-go-lib
203+
go vet -tags cgo,ccalloc ./go/internal/feast ./go/embedded
201204

202205
# Docker
203206

‎docs/reference/feature-servers/go-feature-retrieval.md‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,20 @@ However, some additional dependencies are required for Go <-> Python interoperab
1616
```
1717
pip install feast[go]
1818
```
19+
You will also have to install the apache-arrow c++ libraries, since we use the cgo memory allocator to prevent memory from being incorrectly garbage collected, detailed in these [docs](https://pkg.go.dev/github.com/apache/arrow/go/[email protected]/cdata#ExportArrowRecordBatch).
1920

20-
For developers, if you want to build from source, run `make compile-go-lib` to build and compile the go server.
21+
For macos, run `brew install apache-arrow`.
22+
For linux users, you have to install `libarrow-dev`.
23+
```
24+
sudo apt update
25+
sudo apt install -y -V ca-certificates lsb-release wget
26+
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
27+
sudo apt install -y -V ./apache-arrow-apt-source-latest-$(lsb_release --codename --short).deb
28+
sudo apt update
29+
sudo apt install -y -V libarrow-dev # For C++
30+
```
31+
32+
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.
2133

2234
## Usage
2335

@@ -63,7 +75,7 @@ feature_server:
6375
emit_timeout_micro_secs: 10000
6476
queue_capacity: 10000
6577
```
66-
All these parameters are optional.
78+
All these parameters are optional.
6779

6880
## Future/Current Work
6981

‎go.mod‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,10 @@ module github.com/feast-dev/feast
22

33
go 1.17
44

5-
replace github.com/go-python/gopy v0.4.0 => github.com/feast-dev/gopy v0.4.1-0.20220429180328-4257ac71a4d0
6-
75
require (
86
github.com/apache/arrow/go/v8 v8.0.0
97
github.com/ghodss/yaml v1.0.0
10-
github.com/go-python/gopy v0.4.0
8+
github.com/go-python/gopy v0.4.4
119
github.com/go-redis/redis/v8 v8.11.4
1210
github.com/golang/protobuf v1.5.2
1311
github.com/google/uuid v1.3.0
@@ -28,6 +26,8 @@ require (
2826
github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f // indirect
2927
github.com/goccy/go-json v0.9.6 // indirect
3028
github.com/golang/snappy v0.0.4 // indirect
29+
github.com/gonuts/commander v0.1.0 // indirect
30+
github.com/gonuts/flag v0.1.0 // indirect
3131
github.com/google/flatbuffers v2.0.6+incompatible // indirect
3232
github.com/klauspost/asmfmt v1.3.2 // indirect
3333
github.com/klauspost/compress v1.15.1 // indirect
@@ -38,7 +38,7 @@ require (
3838
github.com/pmezard/go-difflib v1.0.0 // indirect
3939
github.com/zeebo/xxh3 v1.0.2 // indirect
4040
golang.org/x/exp v0.0.0-20220407100705-7b9b53b0aca4 // indirect
41-
golang.org/x/mod v0.6.0-dev.0.20220106191415-9b9b3d81d5e3 // indirect
41+
golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4 // indirect
4242
golang.org/x/net v0.0.0-20220407224826-aac1ed45d8e3 // indirect
4343
golang.org/x/sys v0.0.0-20220412211240-33da011f77ad // indirect
4444
golang.org/x/text v0.3.7 // indirect

‎go.sum‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,6 @@ github.com/envoyproxy/go-control-plane v0.9.10-0.20210907150352-cf90f659a021/go.
8787
github.com/envoyproxy/go-control-plane v0.10.2-0.20220325020618-49ff273808a1/go.mod h1:KJwIaB5Mv44NWtYuAOFCVOjcI94vtpEz2JU/D2v6IjE=
8888
github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c=
8989
github.com/fatih/color v1.7.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5KwzbycvMj4=
90-
github.com/feast-dev/gopy v0.4.1-0.20220429180328-4257ac71a4d0 h1:Go714ObVP1O+a6qK7haXVL28QNm6WMD8bwnN9EA8PlM=
91-
github.com/feast-dev/gopy v0.4.1-0.20220429180328-4257ac71a4d0/go.mod h1:ZO6vpitQ61NVoQP/2yOubPS6ET5pP3CAWCiMYn5eqCc=
9290
github.com/fogleman/gg v1.2.1-0.20190220221249-0403632d5b90/go.mod h1:R/bRT+9gY/C5z7JzPU0zXsXHKM4/ayA+zqcVNZzPa1k=
9391
github.com/fogleman/gg v1.3.0/go.mod h1:R/bRT+9gY/C5z7JzPU0zXsXHKM4/ayA+zqcVNZzPa1k=
9492
github.com/franela/goblin v0.0.0-20200105215937-c9ffbefa60db/go.mod h1:7dvUGVsVBjqR7JHJk0brhHOZYGmfBYOrK0ZhYMEtBr4=
@@ -112,6 +110,8 @@ github.com/go-logfmt/logfmt v0.3.0/go.mod h1:Qt1PoO58o5twSAckw1HlFXLmHsOX5/0LbT9
112110
github.com/go-logfmt/logfmt v0.4.0/go.mod h1:3RMwSq7FuexP4Kalkev3ejPJsZTpXXBr9+V4qmtdjCk=
113111
github.com/go-logfmt/logfmt v0.5.0/go.mod h1:wCYkCAKZfumFQihp8CzCvQ3paCTfi41vtzG1KdI/P7A=
114112
github.com/go-logr/logr v0.4.0/go.mod h1:z6/tIYblkpsD+a4lm/fGIIU9mZ+XfAiaFtq7xTgseGU=
113+
github.com/go-python/gopy v0.4.4 h1:3LTsrfVcmg2VEM6wU+eh4d9EZn5H2iogObXjiQHrF8Q=
114+
github.com/go-python/gopy v0.4.4/go.mod h1:tlA/KcD7rM8B+NQJR4SASwiinfKY0aiMFanHszR8BZA=
115115
github.com/go-redis/redis/v8 v8.11.4 h1:kHoYkfZP6+pe04aFTnhDH6GDROa5yJdHJVNxV3F46Tg=
116116
github.com/go-redis/redis/v8 v8.11.4/go.mod h1:2Z2wHZXdQpCDXEGzqMockDpNyYvi2l4Pxt6RJr792+w=
117117
github.com/go-sql-driver/mysql v1.4.0/go.mod h1:zAC/RDZ24gD3HViQzih4MyKcchzm+sOG5ZlKdlhCg5w=
@@ -147,7 +147,9 @@ github.com/golang/protobuf v1.5.2/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiu
147147
github.com/golang/snappy v0.0.0-20180518054509-2e65f85255db/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q=
148148
github.com/golang/snappy v0.0.4 h1:yAGX7huGHXlcLOEtBnF4w7FQwA26wojNCwOYAEhLjQM=
149149
github.com/golang/snappy v0.0.4/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q=
150+
github.com/gonuts/commander v0.1.0 h1:EcDTiVw9oAVORFjQOEOuHQqcl6OXMyTgELocTq6zJ0I=
150151
github.com/gonuts/commander v0.1.0/go.mod h1:qkb5mSlcWodYgo7vs8ulLnXhfinhZsZcm6+H/z1JjgY=
152+
github.com/gonuts/flag v0.1.0 h1:fqMv/MZ+oNGu0i9gp0/IQ/ZaPIDoAZBOBaJoV7viCWM=
151153
github.com/gonuts/flag v0.1.0/go.mod h1:ZTmTGtrSPejTo/SRNhCqwLTmiAgyBdCkLYhHrAoBdz4=
152154
github.com/google/btree v0.0.0-20180813153112-4030bb1f1f0c/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ=
153155
github.com/google/btree v1.0.0/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ=
@@ -439,8 +441,9 @@ golang.org/x/mod v0.5.1/go.mod h1:5OXOZSfqPIIbmVBIIKWRFfZjPR0E5r58TLhUjH0a2Ro=
439441
golang.org/x/mod v0.5.1/go.mod h1:5OXOZSfqPIIbmVBIIKWRFfZjPR0E5r58TLhUjH0a2Ro=
440442
golang.org/x/mod v0.6.0-dev.0.20211013180041-c96bc1413d57/go.mod h1:3p9vT2HGsQu2K1YbXdKPJLVgG5VJdoTa1poYQBtP1AY=
441443
golang.org/x/mod v0.6.0-dev.0.20211013180041-c96bc1413d57/go.mod h1:3p9vT2HGsQu2K1YbXdKPJLVgG5VJdoTa1poYQBtP1AY=
442-
golang.org/x/mod v0.6.0-dev.0.20220106191415-9b9b3d81d5e3 h1:kQgndtyPBW/JIYERgdxfwMYh3AVStj88WQTlNDi2a+o=
443444
golang.org/x/mod v0.6.0-dev.0.20220106191415-9b9b3d81d5e3/go.mod h1:3p9vT2HGsQu2K1YbXdKPJLVgG5VJdoTa1poYQBtP1AY=
445+
golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4 h1:6zppjxzCulZykYSLyVDYbneBfbaBIQPYMevg0bEwv2s=
446+
golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4=
444447
golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
445448
golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
446449
golang.org/x/net v0.0.0-20180906233101-161cd47e91fd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=

‎go/embedded/online_features.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ func (s *OnlineFeatureService) GetOnlineFeatures(
180180

181181
outputFields := make([]arrow.Field, 0)
182182
outputColumns := make([]arrow.Array, 0)
183-
pool := memory.NewGoAllocator()
183+
pool := memory.NewCgoArrowAllocator()
184184
for _, featureVector := range resp {
185185
outputFields = append(outputFields,
186186
arrow.Field{

0 commit comments

Comments
 (0)