-
Notifications
You must be signed in to change notification settings - Fork 1k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
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]>
- Loading branch information
Showing
12 changed files
with
114 additions
and
27 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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/[email protected] | ||
env: | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -172,12 +172,12 @@ 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/[email protected].0 | ||
go get github.com/go-python/[email protected].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. | ||
go install github.com/go-python/gopy | ||
python -m pip install pybindgen==0.22.0 | ||
python -m pip install pybindgen==0.22.0 protobuf==3.20.1 | ||
|
||
install-protoc-dependencies: | ||
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 | |
python setup.py build_go_protos | ||
|
||
compile-go-lib: install-go-proto-dependencies install-go-ci-dependencies | ||
COMPILE_GO=True python setup.py build_ext --inplace | ||
CGO_LDFLAGS_ALLOW=".*" COMPILE_GO=True python setup.py build_ext --inplace | ||
|
||
# Needs feast package to setup the feature store | ||
test-go: compile-protos-go | ||
install-feast-ci-locally: | ||
pip install -e ".[ci]" | ||
go test ./... | ||
|
||
# Needs feast package to setup the feature store | ||
# CGO flag is due to this issue: https://github.com/golang/go/wiki/InvalidFlag | ||
test-go: compile-protos-go compile-go-lib install-feast-ci-locally | ||
CGO_LDFLAGS_ALLOW=".*" go test -tags cgo,ccalloc ./... | ||
|
||
format-go: | ||
gofmt -s -w go/ | ||
|
||
lint-go: compile-protos-go | ||
go vet ./go/internal/feast ./go/embedded | ||
lint-go: compile-protos-go compile-go-lib | ||
go vet -tags cgo,ccalloc ./go/internal/feast ./go/embedded | ||
|
||
# Docker | ||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,8 +16,20 @@ 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, 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). | ||
|
||
For developers, if you want to build from source, run `make compile-go-lib` to build and compile the go server. | ||
For macos, run `brew install apache-arrow`. | ||
For linux users, you have to install `libarrow-dev`. | ||
``` | ||
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. In order to build the go binaries, you will need to install the `apache-arrow` c++ libraries. | ||
|
||
## Usage | ||
|
||
|
@@ -63,7 +75,7 @@ feature_server: | |
emit_timeout_micro_secs: 10000 | ||
queue_capacity: 10000 | ||
``` | ||
All these parameters are optional. | ||
All these parameters are optional. | ||
|
||
## Future/Current Work | ||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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/[email protected].0"], | ||
subprocess.check_call(["go", "get", "github.com/go-python/[email protected].4"], | ||
env={"PATH": bin_path, **go_env}) | ||
subprocess.check_call(["go", "install", "github.com/go-python/gopy"], | ||
env={"PATH": bin_path, **go_env}) | ||
|
@@ -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, | ||
], | ||
|