Skip to content

[Bug] Clear all the futures when sync is called. - #501

Merged
feast-ci-bot merged 12 commits into
feast-dev:masterfrom
lavkesh:master
Feb 27, 2020
Merged

[Bug] Clear all the futures when sync is called.#501
feast-ci-bot merged 12 commits into
feast-dev:masterfrom
lavkesh:master

Conversation

@lavkesh

@lavkesh lavkesh commented Feb 27, 2020

Copy link
Copy Markdown
Contributor

Futures may contain exception when retry happens. Futures.waitAll() returns when it encounters an exception. This will cause execute() call to never succeed.
So sync() call should clear future List so that retry can proceed.

@woop

woop commented Feb 27, 2020

Copy link
Copy Markdown
Member

/ok-to-test

@woop woop left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Added some comments.

@woop

woop commented Feb 27, 2020

Copy link
Copy Markdown
Member

/approved
/lgtm

@feast-ci-bot

Copy link
Copy Markdown
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lavkesh, woop

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@feast-ci-bot
feast-ci-bot merged commit 50916d5 into feast-dev:master Feb 27, 2020
khorshuheng pushed a commit to khorshuheng/feast that referenced this pull request Mar 17, 2020
* Replacing Jedis With Lettuce in ingestion and serving

* Removing extra lines

* Abstacting redis connection based on store

* Check the connection before connecting as lettuce does the retry automatically

* Running spotless

* Throw Exception if the job store config is null

* Handle No enum constant RuntimeException

* Future should be cleared everytime sync is called
khorshuheng pushed a commit that referenced this pull request Mar 17, 2020
* Replacing Jedis With Lettuce in ingestion and serving

* Removing extra lines

* Abstacting redis connection based on store

* Check the connection before connecting as lettuce does the retry automatically

* Running spotless

* Throw Exception if the job store config is null

* Handle No enum constant RuntimeException

* Future should be cleared everytime sync is called
@ches ches added the kind/bug label May 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants