FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix: Add connection pre-warming for DynamoDB async client by nithin42 · Pull Request #6711 · feast-dev/feast · GitHub

/ feast Public

fix: Add connection pre-warming for DynamoDB async client - #6711

Merged
ntkathole merged 1 commit into
feast-dev:masterfrom
nithin42:fix/dynamodb-warmup-connections-only
Aug 15, 2026
ConversationCommits (1)ChecksFiles changed
Merged

fix: Add connection pre-warming for DynamoDB async client#6711
ntkathole merged 1 commit into
feast-dev:masterfrom
nithin42:fix/dynamodb-warmup-connections-only

Conversation

nithin42 commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

What this PR does / why we need it

The DynamoDB online store async client doesn't establish actual TCP/TLS connections during initialize(), causing the very first get_online_features() call to pay a cold-start overhead (~20ms).
This PR adds a warmup_connections boolean option to DynamoDBOnlineStoreConfig and runs describe_limits() inside a safe try...except block in DynamoDBOnlineStore.initialize() to pre-warm the connections.

Which issue(s) this PR fixes

Fixes #6060

Checks

  • I've made sure the tests are passing.
  • My commits are signed off (git commit -s)
  • My PR title follows conventional commits format

nithin42 requested a review from a team as a code owner August 7, 2026 19:05
This was referenced Aug 7, 2026
Closed
Closed

nithin42 commented Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

@jyejare can you look into fix?

codecov-commenter commented Aug 11, 2026 •
edited
Loading

Copy link
Copy Markdown

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 46.82%. Comparing base (98e5bca) to head (5eb6037).
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files 

@@            Coverage Diff             @@
##           master    #6711      +/-   ##
==========================================
+ Coverage   46.80%   46.82%   +0.01%     
==========================================
  Files         415      415              
  Lines       50399    50406       +7     
  Branches     7214     7215       +1     
==========================================
+ Hits        23591    23601      +10     
+ Misses      25157    25155       -2     
+ Partials     1651     1650       -1
Flag Coverage Δ
go-feature-server 30.58% <ø> (ø)
python-unit 48.14% <100.00%> (+0.01%) ⬆️
Files with missing lines Coverage Δ
sdk/python/feast/infra/online_stores/dynamodb.py 65.34% <100.00%> (+0.84%) ⬆️

... and 1 file with indirect coverage changes

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 98e5bca...5eb6037. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

ntkathole commented Aug 11, 2026

Copy link
Copy Markdown
Member

@nithin42 you can add this new config in documentation specially at Performance tuning guide - DynamoDB tuning section

nithin42 commented Aug 11, 2026

Copy link
Copy Markdown
Contributor Author

@ntkathole Done! Documented the new warmup_connections parameter under the DynamoDB tuning sections in both the Performance Tuning Guide and the DynamoDB reference docs. Also added full unit test coverage for the initial connection failure exception path. Thanks!

nithin42 force-pushed the fix/dynamodb-warmup-connections-only branch from ba7c7bb to cba5604 Compare August 11, 2026 12:35

nithin42 commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

@ntkathole @jyejare Hello! I've addressed your feedback by documenting the new warmup_connections parameter in the tuning guides and reference docs, added unit tests covering the exception handling path, and rebased the branch onto master. All CI checks are now passing green. Could you please take a look and review when you have a moment? Thanks!

ntkathole commented Aug 15, 2026

Copy link
Copy Markdown
Member

@nithin42 I am not able to rebase, please rebase and possibly squash commits

fix: Add connection pre-warming for DynamoDB async client
5eb6037 
Add warmup_connections parameter to DynamoDBOnlineStoreConfig and warm up
connections via describe_limits inside initialize() method to solve cold-start
latency. Also document warmup_connections in the performance tuning guide and
reference docs.

Fixes feast-dev#6060

Signed-off-by: Nithin <kumbam.nithingoud@gmail.com>
nithin42 force-pushed the fix/dynamodb-warmup-connections-only branch from cba5604 to 5eb6037 Compare August 15, 2026 13:14

nithin42 commented Aug 15, 2026

Copy link
Copy Markdown
Contributor Author

@ntkathole Done! Rebased onto the latest master and squashed into a single clean commit.

ntkathole merged commit 89240fa into feast-dev:master Aug 15, 2026
25 checks passed
This file contains hidden or 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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

ntkathole ntkathole approved these changes

Assignees

No one assigned

Labels

ok-to-test

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add connection pre-warming for DynamoDB async client

3 participants

Back | FazBrowse Home | New Git URL