Skip to content

Conversation

@drganjoo
Copy link
Contributor

This PR migrates aws-smithy-http-server from [email protected]/[email protected] to [email protected]/[email protected].

Background

This is the second PR in the HTTP 1.x migration series:

  1. ✅ Legacy crates PR - Added aws-smithy-legacy-http and aws-smithy-legacy-http-server for [email protected] support
  2. 👉 This PR - Upgrade aws-smithy-http-server to [email protected]/[email protected]
  3. 🔜 Codegen changes - Add http-1x flag and conditional dependency logic

Key Changes

1. Dependency Upgrades

  • http: 0.2.91.x
  • http-body: 0.4.51.0
  • hyper: 0.14.261.x (with server, http1, http2 features)
  • hyper-util: Added 0.1 for server utilities (server-auto, server-graceful, etc.)
  • http-body-util: Added 0.1 for body utilities
  • tower-http: 0.30.6
  • lambda_http: 0.8.40.17 (for AWS Lambda support)

Updated aws-smithy-types to use http-body-1-x feature instead of http-body-0-4-x.

2. New serve Module

Added a comprehensive serve module (inspired by axum::serve) that provides:

  • Simple server API: Ergonomic serve(listener, service) function
  • Flexible listeners: Listener trait supporting TCP, Unix sockets, and custom transports
  • Connection limiting: Built-in limit_connections() via ListenerExt
  • Graceful shutdown: Zero-cost when unused, opt-in with .with_graceful_shutdown()
  • Hyper customization: .configure_hyper() for protocol and performance tuning
  • Comprehensive docs: Examples, troubleshooting, and advanced patterns

Files: src/serve/mod.rs (794 lines), src/serve/listener.rs (269 lines)

3. API Updates Throughout

Updated all modules to work with [email protected] APIs:

  • body.rs: Enhanced to work with [email protected] traits (+176 lines)
  • routing/lambda_handler.rs: Updated for [email protected] (+299 lines)
  • layer/alb_health_check.rs: Adapted for new HTTP types (+150 lines)
  • protocol modules: Updated all protocol implementations (REST JSON, AWS JSON, REST XML, RPC v2 CBOR)
  • rejection/error types: Added implementations for [email protected] error types

4. Comprehensive Testing

  • serve_integration_test.rs: 812 lines of integration tests for the serve module
  • graceful_shutdown_test.rs: 211 lines testing graceful shutdown scenarios
  • Tests verify HTTP/1, HTTP/2, connection limits, graceful shutdown, and error handling

Migration Impact

Breaking Change: This updates the public API to use [email protected] types (http::Request, http::Response, etc.).

Users will need to:

  1. Update their dependencies to use [email protected]
  2. Use the new serve() API or update to [email protected] connection APIs
  3. Update any code working directly with HTTP types

Benefits

  • Modern ecosystem: Aligns with the latest Rust HTTP ecosystem
  • Better ergonomics: New serve module provides simpler server setup
  • Performance: Benefits from [email protected] performance improvements
  • Maintainability: No need to maintain compatibility with deprecated versions
  • Feature completeness: Graceful shutdown, connection limiting, and flexible configuration

Testing

  • ✅ Integration tests cover major scenarios
  • ✅ Graceful shutdown tested
  • ✅ All protocol implementations verified

Next Steps

The next PR will update the codegen to conditionally generate code that uses either:

@drganjoo drganjoo requested a review from a team as a code owner October 31, 2025 22:00
@drganjoo drganjoo requested a review from a team as a code owner November 1, 2025 09:13
uuid = { version = "1.1.2", features = ["v4", "fast-rng"], optional = true }

[dev-dependencies]
aws-smithy-legacy-http-server = { path = "../aws-smithy-legacy-http-server" }
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm sure I'll find out but...why do we have this dependency?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This dependency along with its benchmark, needs to be removed. While it was initially added to include some AI-generated benchmarking, the generated benchmark needs significant improvement. However, I'd prefer not to delay this PR further by waiting for a proper benchmark framework to be implemented.

Comment on lines 280 to 306
println!("════════════════════════════════════════════════════════════");
println!();
println!("📊 Summary");
println!("────────────────────────────────────────────────────────────");
println!();

let speedup = new_results.requests_per_sec / legacy_results.requests_per_sec;
let latency_improvement = (legacy_results.avg_latency_micros - new_results.avg_latency_micros)
/ legacy_results.avg_latency_micros * 100.0;

println!(" Legacy (hyper 0.14): {:.2} req/s", legacy_results.requests_per_sec);
println!(" New (hyper 1.x): {:.2} req/s", new_results.requests_per_sec);
println!();
println!(" Throughput change: {:.2}% ({:.2}x)", (speedup - 1.0) * 100.0, speedup);
println!(" Latency improvement: {:.2}%", latency_improvement);
println!();

if speedup > 1.01 {
println!(" ✅ New server is faster!");
} else if speedup < 0.99 {
println!(" ⚠️ Legacy server was faster");
} else {
println!(" ≈ Performance is similar");
}
println!();
println!("════════════════════════════════════════════════════════════");

Copy link
Collaborator

Choose a reason for hiding this comment

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

this is actually...a scientific comparison / load test or anything

// The listener limits concurrent connections to 100.
// Once 100 connections are active, new connections will wait at the OS level
// until an existing connection completes.
let listener = TcpListener::bind("0.0.0.0:3000").await?.limit_connections(100);
Copy link
Collaborator

Choose a reason for hiding this comment

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

huh...I wonder if we want an extension trait here or something more explicit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It comes from ListenerExt imported at the top—happy to hear what you had in mind for making it more explicit.


// Limit concurrent requests at the application level
let app = ServiceBuilder::new()
.layer(ConcurrencyLimitLayer::new(50))
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should NEVER have examples of ConcurrencyLimit that are not combined with LoadShed because it leads to memory / connection leaks otherwise.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're absolutely right about this. I'll remove the example entirely—the intent was to show how to apply a Tower layer, but this topic really belongs in a dedicated production hardening guide, not as a standalone example suggesting there's a 'one size fits all' solution.

Comment on lines 6 to 19
//! Example comparing handle_connection vs handle_connection_strategy performance.
//!
//! This example starts a simple HTTP server that responds with "Hello, World!"
//! and can be load tested with tools like `oha`.
//!
//! Usage:
//! # Run with strategy pattern implementation (uses handle_connection_strategy)
//! cargo run --example serve_comparison --release
//!
//! # Then in another terminal, run load test:
//! oha -z 30s -c 100 http://127.0.0.1:3000/
//!
//! To test the original branching implementation, you would need to modify
//! the serve module to export handle_connection and use it here.
Copy link
Collaborator

Choose a reason for hiding this comment

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

this doesn't appear to be comparing anything?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, the name is misleading—it's not actually comparing anything. This was just being used to test the server independently against workloads using oha/wrk. I'll remove it.

Comment on lines +6 to +9
#![cfg_attr(
not(feature = "request-id"),
allow(unused_imports, dead_code, unreachable_code)
)]
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you can cfg out the entire example? Or can you not because its an example and not a module?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants