Issue 424: Add support for handling of vector fields.

[dgoldenberg-ias]

Description

This change will allow the users of opensearch-hadoop to get values of fields of type knn-vector to be loaded into a Spark dataframe properly from OpenSearch or persisted to OpenSearch from a Spark dataframe. Up till now, the type had not yet gotten added to the codebase and thus, for example, one of the effects was that knn-vector field values were simply not returned as a column in the Spark dataframe. With this change, they will be.

Issues Resolved

Closes #424

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[Xtansia]

Thanks for contributing this @dgoldenberg-ias, this is great!

There's just a compilation error at the moment as it looks like you've missed adding the enum variant to FieldType

[dgoldenberg-ias]

Thanks for contributing this @dgoldenberg-ias, this is great!

There's just a compilation error at the moment as it looks like you've missed adding the enum variant to FieldType

Great catch @Xtansia, thank you! :) I was going between a couple of my branches and this slipped through the cracks. Should be good now.

[dgoldenberg-ias]

Thanks for contributing this @dgoldenberg-ias, this is great!

There's just a compilation error at the moment as it looks like you've missed adding the enum variant to FieldType

Hi @Xtansia, I was wondering if the PR is good to go now. Let me know pls, thanks.

[Xtansia]

Thanks for contributing this @dgoldenberg-ias, this is great!
There's just a compilation error at the moment as it looks like you've missed adding the enum variant to FieldType

Hi @Xtansia, I was wondering if the PR is good to go now. Let me know pls, thanks.

Sorry for the delay! Changes look good, would you be able to add a short test like these geo_point ones here and here?

[dgoldenberg-ias]

Thanks for contributing this @dgoldenberg-ias, this is great!
There's just a compilation error at the moment as it looks like you've missed adding the enum variant to FieldType

Hi @Xtansia, I was wondering if the PR is good to go now. Let me know pls, thanks.

Sorry for the delay! Changes look good, would you be able to add a short test like these geo_point ones here and here?

Hi @Xtansia, sure I can do that. The tests look the same but I get it, we want coverage for both spark sql 20 and 30..

[dgoldenberg-ias]

Thanks for contributing this @dgoldenberg-ias, this is great!
There's just a compilation error at the moment as it looks like you've missed adding the enum variant to FieldType

Hi @Xtansia, I was wondering if the PR is good to go now. Let me know pls, thanks.

Sorry for the delay! Changes look good, would you be able to add a short test like these geo_point ones here and here?

Hi @Xtansia, I've added two unit tests, as discussed. However, I've started experiencing build problems. Could you lend a hand? I can email you more details but basically below are the two main issues I'm seeing:

I run ./gradlew clean distZip -Dbuild.snapshot=false.

I get the below error:

FAILURE: Build failed with an exception.

* What went wrong:
Could not determine the dependencies of task ':qa:kerberos:hadoopFixture#datanode.extract'.
> Could not resolve all files for configuration ':qa:kerberos:downloadHadoop#3.3.2'.
   > Could not resolve hadoop.common:hadoop-3.3.2:hadoop-3.3.2.
     Required by:
         project :qa:kerberos
      > Could not resolve hadoop.common:hadoop-3.3.2:hadoop-3.3.2.
         > Could not get resource 'https://repo.spring.io/plugins-release-local/hadoop/common/hadoop-3.3.2/hadoop-3.3.2/hadoop-3.3.2-hadoop-3.3.2.pom'.
            > Could not GET 'https://repo.spring.io/plugins-release-local/hadoop/common/hadoop-3.3.2/hadoop-3.3.2/hadoop-3.3.2-hadoop-3.3.2.pom'. Received status code 401 from server: 
      > Could not resolve hadoop.common:hadoop-3.3.2:hadoop-3.3.2.
         > Could not get resource 'https://artifacts.opensearch.org/snapshots/hadoop/common/hadoop-3.3.2/hadoop-3.3.2/hadoop-3.3.2-hadoop-3.3.2.pom'.
            > Could not GET 'https://artifacts.opensearch.org/snapshots/hadoop/common/hadoop-3.3.2/hadoop-3.3.2/hadoop-3.3.2-hadoop-3.3.2.pom'. Received status code 403 from server: Forbidden
      > Could not resolve hadoop.common:hadoop-3.3.2:hadoop-3.3.2.
         > Could not get resource 'https://snapshots.opensearch.org/releases/core/opensearch/hadoop-3.3.2/hadoop-3.3.2-min-hadoop-3.3.2.xml'.
            > Could not GET 'https://snapshots.opensearch.org/releases/core/opensearch/hadoop-3.3.2/hadoop-3.3.2-min-hadoop-3.3.2.xml'.
               > snapshots.opensearch.org: nodename nor servname provided, or not known
      > Could not resolve hadoop.common:hadoop-3.3.2:hadoop-3.3.2.
         > Could not get resource 'https://artifacts.opensearch.org/releases/core/opensearch/hadoop-3.3.2/hadoop-3.3.2-min-hadoop-3.3.2.xml'.
            > Could not GET 'https://artifacts.opensearch.org/releases/core/opensearch/hadoop-3.3.2/hadoop-3.3.2-min-hadoop-3.3.2.xml'. Received status code 403 from server: Forbidden

I temporarily commented out these lines in settings.gradle:

include 'qa'
include 'qa:kerberos'

Then all the integration tests fail, like this:

> Task :opensearch-spark:integrationTest FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':opensearch-spark:integrationTest'.
> Could not resolve all files for configuration ':opensearch-spark:opensearch_distro_extracted_testclusters-opensearch-spark-integTest-0-2.13.0-SNAPSHOT-'.
   > Could not resolve opensearch-distribution-snapshot:opensearch:2.13.0-SNAPSHOT.
     Required by:
         project :opensearch-spark
      > Could not resolve opensearch-distribution-snapshot:opensearch:2.13.0-SNAPSHOT.
         > Could not get resource 'https://artifacts.opensearch.org/snapshots/core/opensearch/2.13.0-SNAPSHOT/opensearch-min-2.13.0-SNAPSHOT-darwin-arm64-latest.tar.gz'.
            > Could not HEAD 'https://artifacts.opensearch.org/snapshots/core/opensearch/2.13.0-SNAPSHOT/opensearch-min-2.13.0-SNAPSHOT-darwin-arm64-latest.tar.gz'. Received status code 403 from server: Forbidden

Then I can skip the integration tests OK (using -x) but that's really not what I want of course, I'd like the build to work fully as before. I have seen these errors before. Curiously, they had stopped occurring but now they've started showing up again. I wonder if it's something in my environment.

I have:

• Gradle 8.8
  and:

export HADOOP_HOME=/Users/dgoldenberg/tech/hadoop-3.3.2
export SPARK_HOME=/Users/dgoldenberg/tech/spark-3.5.1-bin-hadoop3

export SCALA_HOME=/opt/homebrew/opt/scala@2.13
export PATH="/opt/homebrew/opt/scala@2.13/bin:$PATH"

export JAVA11_HOME=/Library/Java/JavaVirtualMachines/jdk-11.jdk/Contents/Home
export JAVA17_HOME=/opt/homebrew/opt/openjdk@17/libexec/openjdk.jdk/Contents/Home
export JAVA8_HOME=/Library/Java/JavaVirtualMachines/adoptopenjdk-8.jdk/Contents/Home

# export JAVA_HOME=$JAVA17_HOME
export JAVA_HOME=$JAVA11_HOME
# export JAVA_HOME=$JAVA8_HOME

It seems setting the default Java to 8 or 11 should be OK (?)
Is there anything in this env that stands out to you as out of kilter?

Also, is the PR now all set and can it be merged?
Thanks!

[dgoldenberg-ias]

Hi @Xtansia,

Any word on the build failure?

Looking at https://github.com/opensearch-project/opensearch-hadoop/actions/runs/10022839453/job/27729806497#step:10:437,

* What went wrong:
Execution failed for task ':opensearch-spark-30:integrationTest'.
> There were failing tests. See the results at: file:///home/runner/work/opensearch-hadoop/opensearch-hadoop/spark/sql-30/build/test-results/integrationTest/

> Task :opensearch-spark-30:integrationTest FAILED

Also would appreciate some help on the local build. Thanks.

[Xtansia]

@dgoldenberg-ias The CI jobs are failing as the default OpenSearch cluster used for integration testing does not have the knn plugin installed. I am looking into the best way to handle this.

I am currently unable to reproduce your issues fetching dependencies locally, it might have been a transient issue with our snapshots maven repository.

Additionally it looks like some of your commits are missing the final Signed-off-by: line causing the DCO check to fail, you should be able to resolve this by doing something like git rebase HEAD~6 --signoff as suggested here: https://github.com/opensearch-project/opensearch-hadoop/pull/489/checks?check_run_id=27703060874

[dgoldenberg-ias]

Hi @Xtansia

Thanks for looking into the install of the knn plugin.

I have added the necessary Signed-off-by's.

issues fetching dependencies locally, it might have been a transient issue with our snapshots maven repository.

Yes, this seems intermittent and it does seem to be something to do with the snapshots maven repo.

[dgoldenberg-ias]

Hi @Xtansia, I was just wondering if you've had a chance to take a look at the install of the knn plugin. Anything I could perhaps help with? Let me know, thanks.

[dgoldenberg-ias]

Hi @Xtansia, I was just wondering if you've had a chance to take a look at the install of the knn plugin. Anything I could perhaps help with? Let me know, thanks. It would be great to wrap this up :)

[Xtansia]

Hi @dgoldenberg-ias, very sorry for the delay, I've been swamped lately. I had a quick look into getting the k-nn plugin setup in the integ tests and doesn't look like a super simple job. For now I think easiest to get this merged we can just mark the tests as @Ignore("k-NN plugin is currently missing") and can follow up in future.

[dgoldenberg-ias]

Hi @dgoldenberg-ias, very sorry for the delay, I've been swamped lately. I had a quick look into getting the k-nn plugin setup in the integ tests and doesn't look like a super simple job. For now I think easiest to get this merged we can just mark the tests as @Ignore("k-NN plugin is currently missing") and can follow up in future.

Hi @Xtansia, thanks for getting back to me!
Sry, can you please outline which tests you think should be @ Ignore'd. All tests seem to be passing for me but I might be missing some angle of this. Would appreciate a bit more hand-holding :). Might it be worth filing a separate ticket for this plugin (if so, maybe a few more details on where this needs to be added).

My other question for you was, will this PR become a part of the next release; do you know what the expected/planned schedule for it is?

[Xtansia]

Hi @dgoldenberg-ias, very sorry for the delay, I've been swamped lately. I had a quick look into getting the k-nn plugin setup in the integ tests and doesn't look like a super simple job. For now I think easiest to get this merged we can just mark the tests as @Ignore("k-NN plugin is currently missing") and can follow up in future.

Hi @Xtansia, thanks for getting back to me! Sry, can you please outline which tests you think should be @ Ignore'd. All tests seem to be passing for me but I might be missing some angle of this. Would appreciate a bit more hand-holding :). Might it be worth filing a separate ticket for this plugin (if so, maybe a few more details on where this needs to be added).

My other question for you was, will this PR become a part of the next release; do you know what the expected/planned schedule for it is?

I think the two tests added in this PR will need it to pass CI. Could you please rebase onto the latest main and re-push your branch to re-trigger the CI runs to confirm first? I'll then open an issue regarding fixing the integ tests/k-nn tests.

Yes it would become part of the next release, we don't have a fixed release schedule, but I could look into kicking one off in the next week.

[dgoldenberg-ias]

Hi @Xtansia,

Could you please rebase onto the latest main and re-push your branch to re-trigger the CI runs to confirm first?

A rebase did not rake anything in. I just added a couple of comments in this latest commit. Could you let me know how the CI went? and whether those @ Ignore's are necessary. We could reference the new ticket in those although I don't see a pattern for that in other @ Ignore's.

I'll then open an issue regarding fixing the integ tests/k-nn tests.

Great.

Yes it would become part of the next release, we don't have a fixed release schedule, but I could look into kicking one off in the next week.

Perfect, thanks.

[dgoldenberg-ias]

Hi @Xtansia,

The build logs seem a bit cryptic. But there are build failures in tests of spark 20 and spark 30. Safe to assume we just need an @ Ignore on testKnnVectorAsArrayOfFloats()? and that the mappingTest on the Java side is OK?

[Xtansia]

@dgoldenberg-ias I've fixed the broken CI in 766ae02, could you please rebase ontop of that?

[dgoldenberg-ias]

@dgoldenberg-ias I've fixed the broken CI in 766ae02, could you please rebase ontop of that?

@Xtansia I'm not able to; it's really odd.

git checkout main
git pull
Already up to date.

git log -1
commit cd3db19a6137efc58b109d54cd5df8dc678e6e20 (HEAD -> main, origin/main, origin/HEAD)
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Tue Jul 2 11:02:50 2024 +1200

Somehow I can only get to that commit from Jul 2 and it's not seeing yours. Any ideas?

[Xtansia]

@dgoldenberg-ias If you navigate to your fork, you should see something like below, click sync fork to bring in the latest changes from this repo and then do the rebase:

[dgoldenberg-ias]

@Xtansia Done, I believe :)

[Xtansia]

@dgoldenberg-ias It is just the testKnnVectorAsArrayOfFloats tests that need an @Ignore added. Also btw it appears you've rebased the main changes on-top of your branch, rather than rebasing your branch on-top of main

[dgoldenberg-ias]

@dgoldenberg-ias It is just the testKnnVectorAsArrayOfFloats tests that need an @Ignore added. Also btw it appears you've rebased the main changes on-top of your branch, rather than rebasing your branch on-top of main

Hi @Xtansia,

• I've added the @ignore's to the two tests
• "you've rebased the main changes on-top of your branch" - sry, long days here.. can you outline the commands, I keep doing git checkout main; git pull; git checkout issue-424-support-knn-vectors; git rebase origin/main. Where is the direction getting reversed?

[Xtansia]

I'm not sure where it's gone wrong as in theory that should work. You should be able to fixup your branch like so:

# Move most recent commit back to original commits, dropping rebased main commits
git rebase --onto faf01c9ad8eac0222ece9c872b38fee541a4e453 HEAD~1
# Rebase all PR commits onto main
git rebase --onto main $(git merge-base main issue-424-support-knn-vectors)

[dgoldenberg-ias]

I'm not sure where it's gone wrong as in theory that should work. You should be able to fixup your branch like so:

# Move most recent commit back to original commits, dropping rebased main commits
git rebase --onto faf01c9ad8eac0222ece9c872b38fee541a4e453 HEAD~1
# Rebase all PR commits onto main
git rebase --onto main $(git merge-base main issue-424-support-knn-vectors)

Hi Thomas @Xtansia , thanks for that. I did that but things still aren't looking right in the modified files list in the PR.

I don't grok why git keeps wanting me to merge CHANGELOG.md...

Here's the details of what went on:

---

git rebase --onto faf01c9 HEAD~1
Successfully rebased and updated refs/heads/issue-424-support-knn-vectors.

git rebase --onto main $(git merge-base main issue-424-support-knn-vectors)
dropping 4e52d07 Issue 424: Add support for handling of vector fields. Signed-off-by: Dmitry Goldenberg dgoldenberg@integralads.com -- patch contents already upstream
Auto-merging CHANGELOG.md
CONFLICT (content): Merge conflict in CHANGELOG.md
error: could not apply 2340c76... Issue 424: Add support for handling of vector fields. Added a line to the CHANGELOG file. Signed-off-by: Dmitry Goldenberg dgoldenberg@integralads.com
hint: Resolve all conflicts manually, mark them as resolved with
hint: "git add/rm <conflicted_files>", then run "git rebase --continue".
hint: You can instead skip this commit: run "git rebase --skip".
hint: To abort and get back to the state before "git rebase", run "git rebase --abort".
Could not apply 2340c76... Issue 424: Add support for handling of vector fields. Added a line to the CHANGELOG file. Signed-off-by: Dmitry Goldenberg dgoldenberg@integralads.com

vi CHANGELOG.md
~/tech/opensearch-hadoop >> git add CHANGELOG.md
~/tech/opensearch-hadoop >> git rebase --continue
dropping cc2eff3 Issue 424: Add support for handling of vector fields. Added the PR number to the CHANGELOG file. -- patch contents already upstream
dropping 52b5302 Issue 424: Add support for handling of vector fields. Added the missing enum variant for KNN_VECTOR to the FieldType enum. -- patch contents already upstream
dropping 02fae97 Issue 424: Add support for handling of vector fields. Signed-off-by: Dmitry Goldenberg dgoldenberg@integralads.com -- patch contents already upstream
Successfully rebased and updated refs/heads/issue-424-support-knn-vectors.
~/tech/opensearch-hadoop >> git status
On branch issue-424-support-knn-vectors
Your branch and 'origin/issue-424-support-knn-vectors' have diverged,
and have 35 and 33 different commits each, respectively.
(use "git pull" to merge the remote branch into yours)

nothing to commit, working tree clean
~/tech/opensearch-hadoop >> git pull
warning: skipped previously applied commit 5ef87da
warning: skipped previously applied commit be87808
warning: skipped previously applied commit d137a2b
warning: skipped previously applied commit 1216683
warning: skipped previously applied commit 0ef9f35
warning: skipped previously applied commit dfd7096
warning: skipped previously applied commit 7fa224f
warning: skipped previously applied commit c7f5a6d
warning: skipped previously applied commit 34684d2
warning: skipped previously applied commit 354d30c
warning: skipped previously applied commit 75cbb5c
warning: skipped previously applied commit 55a930e
warning: skipped previously applied commit de92af9
warning: skipped previously applied commit 1265716
warning: skipped previously applied commit 027897c
warning: skipped previously applied commit ed19453
warning: skipped previously applied commit 1778d59
warning: skipped previously applied commit f5853c8
warning: skipped previously applied commit 3cd19cf
warning: skipped previously applied commit 2191387
warning: skipped previously applied commit 917981a
warning: skipped previously applied commit e366add
warning: skipped previously applied commit 76cddac
warning: skipped previously applied commit 766ae02
warning: skipped previously applied commit 30a2398a
warning: skipped previously applied commit 77743457
warning: skipped previously applied commit b8bed716
warning: skipped previously applied commit ec0b2788
warning: skipped previously applied commit 5885320d
warning: skipped previously applied commit 49137c6d
warning: skipped previously applied commit 40b03d7d
warning: skipped previously applied commit 883a256d
hint: use --reapply-cherry-picks to include skipped commits
hint: Disable this message with "git config advice.skippedCherryPicks false"
dropping fbd9d31 Remove unnecessary outputCommitter setting (#465) -- patch contents already upstream
Auto-merging CHANGELOG.md
CONFLICT (content): Merge conflict in CHANGELOG.md
error: could not apply 4576f390... Issue 424: Add support for handling of vector fields. Added a line to the CHANGELOG file. Signed-off-by: Dmitry Goldenberg dgoldenberg@integralads.com
hint: Resolve all conflicts manually, mark them as resolved with
hint: "git add/rm <conflicted_files>", then run "git rebase --continue".
hint: You can instead skip this commit: run "git rebase --skip".
hint: To abort and get back to the state before "git rebase", run "git rebase --abort".
Could not apply 4576f390... Issue 424: Add support for handling of vector fields. Added a line to the CHANGELOG file. Signed-off-by: Dmitry Goldenberg dgoldenberg@integralads.com
~/tech/opensearch-hadoop >> vi CHANGELOG.md
(reverse-i-search)`-a': ls -al
~/tech/opensearch-hadoop >> git add CHANGELOG.md
~/tech/opensearch-hadoop >> git rebase --continue
Successfully rebased and updated refs/heads/issue-424-support-knn-vectors.
~/tech/opensearch-hadoop >> git status
On branch issue-424-support-knn-vectors
Your branch is ahead of 'origin/issue-424-support-knn-vectors' by 1 commit.
(use "git push" to publish your local commits)

nothing to commit, working tree clean
~/tech/opensearch-hadoop >> git push
Enumerating objects: 35, done.
Counting objects: 100% (35/35), done.
Delta compression using up to 10 threads
Compressing objects: 100% (15/15), done.
Writing objects: 100% (21/21), 2.16 KiB | 1.08 MiB/s, done.
Total 21 (delta 9), reused 0 (delta 0), pack-reused 0
remote: Resolving deltas: 100% (9/9), completed with 7 local objects.
To github.com:dgoldenberg-ias/opensearch-hadoop.git
5d9dabc..9379374 issue-424-support-knn-vectors -> issue-424-support-knn-vectors
~/tech/opensearch-hadoop >> git status
On branch issue-424-support-knn-vectors
Your branch is up to date with 'origin/issue-424-support-knn-vectors'.

nothing to commit, working tree clean

[Xtansia]

A couple things:

1. I think your local main does not match remote.
2. After my mentioned fix you should have just git push --force rather than git pull --rebase as that just brings back the polluted branch. Sorry I should have specified that.

Let's try the following:

# Stash any un-commited changes to be safe
git stash -u

# Bring in up-to-date remote info
git fetch origin

# Switch to main branch and match it to remote
git switch main
git reset --hard origin/main

# Switch to feature branch and ensure it matches remote
git switch issue-424-support-knn-vectors
git reset --hard origin/issue-424-support-knn-vectors

# Undo duplicated commit that adds second copy of tests
git reset --hard HEAD~1

# Move most recent commit back to original commits, dropping rebased main commits
git rebase --onto faf01c9ad8eac0222ece9c872b38fee541a4e453 HEAD~1

# Rebase all PR commits onto main
git rebase --onto origin/main $(git merge-base origin/main issue-424-support-knn-vectors)

# Push to remote
git push -u -f origin issue-424-support-knn-vectors

[dgoldenberg-ias]

That is great, thank you so much @Xtansia. It looks like we're at a good, clean state now.

Is this good to go or is anything else needed? I see the PR saying "12 workflows awaiting approval".

[dgoldenberg-ias]

Thanks for all the help, @Xtansia, much appreciated!

I assume you'll be merging this into main?

Could you let me know about the new release? (dgoldenberg@integralads.com)

[Xtansia]

@dgoldenberg-ias I recommend "watching" the repo to be notified.

1. Click on "Watch" on the repository home page:
   

2. Click on "Custom":
   

3. Select "Releases":
   

[dgoldenberg-ias]

Got it, @Xtansia, thanks for the detailed explanation :)

Please let me know if there are other units of work I could pick up; I'd love to contribute more. Cheers!

[Xtansia]

Got it, @Xtansia, thanks for the detailed explanation :)

Please let me know if there are other units of work I could pick up; I'd love to contribute more. Cheers!

Any and all contributions would be greatly appreciated! Feel free to have a look through the open issues if there's something there that peaks your interest, or any other improvements/fixes you can think of yourself.

[dgoldenberg-ias]

Hi @Xtansia, we're looking to cut a release soon and were hoping to reference this next upcoming release of openseach-hadoop. Any word on when the next release will be out? Thanks.

[Xtansia]

@dgoldenberg-ias v1.3.0 has now been released

[dgoldenberg-ias]

@dgoldenberg-ias v1.3.0 has now been released

Awesome! Thanks, @Xtansia