updates

Author: julienrbrt

apps/testapp/kv/bench/main.go

@@ -323,9 +323,9 @@ func printResults(elapsed time.Duration, workers, batchSize, total, success, fai
 		fmt.Printf(rowFmt+"\n", goalLabel, "REACHED")
 		fmt.Println(sep)
 		fmt.Println()
-		fmt.Println("  ====================================================")
-		fmt.Printf("      S U C C E S S !   %s  T X / S  R E A C H E D !\n", formatNum(targetRPS))
-		fmt.Println("  ====================================================")
+		fmt.Println("====================================================")
+		fmt.Printf(" S U C C E S S !   %s  T X / S  R E A C H E D !\n", formatNum(targetRPS))
+		fmt.Println("====================================================")
 	} else {
 		fmt.Printf(rowFmt+"\n", goalLabel, "NOT REACHED")
 		fmt.Println(sep)

apps/testapp/kv/http_server.go

@@ -12,16 +12,17 @@ import (
 	"time"
 
 	ds "github.com/ipfs/go-datastore"
+	"github.com/ipfs/go-datastore/query"
 )
 
-var acceptedResp = []byte("Transaction accepted")
-
+// HTTPServer wraps a KVExecutor and provides an HTTP interface for it
 type HTTPServer struct {
 	executor    *KVExecutor
 	server      *http.Server
 	injectedTxs atomic.Uint64
 }
 
+// NewHTTPServer creates a new HTTP server for the KVExecutor
 func NewHTTPServer(executor *KVExecutor, listenAddr string) *HTTPServer {
 	hs := &HTTPServer{
 		executor: executor,
@@ -38,13 +39,14 @@ func NewHTTPServer(executor *KVExecutor, listenAddr string) *HTTPServer {
 		Addr:              listenAddr,
 		Handler:           mux,
 		ReadHeaderTimeout: 10 * time.Second,
-		MaxHeaderBytes:    4096,
 	}
 
 	return hs
 }
 
+// Start begins listening for HTTP requests
 func (hs *HTTPServer) Start(ctx context.Context) error {
+	// Start the server in a goroutine
 	errCh := make(chan error, 1)
 	go func() {
 		fmt.Printf("KV Executor HTTP server starting on %s\n", hs.server.Addr)
@@ -53,8 +55,10 @@ func (hs *HTTPServer) Start(ctx context.Context) error {
 		}
 	}()
 
+	// Monitor for context cancellation
 	go func() {
 		<-ctx.Done()
+		// Create a timeout context for shutdown
 		shutdownCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
 		defer cancel()
 
@@ -64,30 +68,42 @@ func (hs *HTTPServer) Start(ctx context.Context) error {
 		}
 	}()
 
+	// Check if the server started successfully
 	select {
 	case err := <-errCh:
 		return err
-	case <-time.After(100 * time.Millisecond):
+	case <-time.After(100 * time.Millisecond): // Give it a moment to start
+		// Server started successfully
 		return nil
 	}
 }
 
+// Stop shuts down the HTTP server
 func (hs *HTTPServer) Stop() error {
 	return hs.server.Close()
 }
 
+// handleTx handles transaction submissions
+// POST /tx with raw binary data or text in request body
+// It is recommended to use transactions in the format "key=value" to be consistent
+// with the KVExecutor implementation that parses transactions in this format.
+// Example: "mykey=myvalue"
 func (hs *HTTPServer) handleTx(w http.ResponseWriter, r *http.Request) {
 	if r.Method != http.MethodPost {
 		http.Error(w, "Method not allowed", http.StatusMethodNotAllowed)
 		return
 	}
 
-	body, err := io.ReadAll(io.LimitReader(r.Body, 4096))
-	r.Body.Close()
+	body, err := io.ReadAll(r.Body)
 	if err != nil {
 		http.Error(w, "Failed to read request body", http.StatusBadRequest)
 		return
 	}
+	defer func() {
+		if err := r.Body.Close(); err != nil {
+			fmt.Printf("Error closing request body: %v\n", err)
+		}
+	}()
 
 	if len(body) == 0 {
 		http.Error(w, "Empty transaction", http.StatusBadRequest)
@@ -97,7 +113,10 @@ func (hs *HTTPServer) handleTx(w http.ResponseWriter, r *http.Request) {
 	hs.executor.InjectTx(body)
 	hs.injectedTxs.Add(1)
 	w.WriteHeader(http.StatusAccepted)
-	w.Write(acceptedResp)
+	_, err = w.Write([]byte("Transaction accepted"))
+	if err != nil {
+		fmt.Printf("Error writing response: %v\n", err)
+	}
 }
 
 func (hs *HTTPServer) handleTxBatch(w http.ResponseWriter, r *http.Request) {
@@ -160,39 +179,77 @@ func (r *bytesReaderImpl) Read(p []byte) (int, error) {
 	return n, nil
 }
 
+// handleKV handles direct key-value operations (GET/POST) against the database
+// GET /kv?key=somekey - retrieve a value
+// POST /kv with JSON {"key": "somekey", "value": "somevalue"} - set a value
 func (hs *HTTPServer) handleKV(w http.ResponseWriter, r *http.Request) {
-	if r.Method != http.MethodGet {
-		http.Error(w, "Method not allowed", http.StatusMethodNotAllowed)
-		return
-	}
+	switch r.Method {
+	case http.MethodGet:
+		key := r.URL.Query().Get("key")
+		if key == "" {
+			http.Error(w, "Missing key parameter", http.StatusBadRequest)
+			return
+		}
 
-	key := r.URL.Query().Get("key")
-	if key == "" {
-		http.Error(w, "Missing key parameter", http.StatusBadRequest)
-		return
-	}
+		// Use r.Context() when calling the executor method
+		value, exists := hs.executor.GetStoreValue(r.Context(), key)
+		if !exists {
+			// GetStoreValue now returns false on error too, check logs for details
+			// Check if the key truly doesn't exist vs a DB error occurred.
+			// For simplicity here, we treat both as Not Found for the client.
+			// A more robust implementation might check the error type.
+			_, err := hs.executor.db.Get(r.Context(), ds.NewKey(key))
+			if errors.Is(err, ds.ErrNotFound) {
+				http.Error(w, "Key not found", http.StatusNotFound)
+			} else {
+				// Some other DB error occurred
+				http.Error(w, "Failed to retrieve key", http.StatusInternalServerError)
+				fmt.Printf("Error retrieving key '%s' from DB: %v\n", key, err)
+			}
+			return
+		}
 
-	value, exists := hs.executor.GetStoreValue(r.Context(), key)
-	if !exists {
-		if _, err := hs.executor.db.Get(r.Context(), ds.NewKey(key)); errors.Is(err, ds.ErrNotFound) {
-			http.Error(w, "Key not found", http.StatusNotFound)
-		} else {
-			http.Error(w, "Failed to retrieve key", http.StatusInternalServerError)
-			fmt.Printf("Error retrieving key '%s' from DB: %v\n", key, err)
+		_, err := w.Write([]byte(value))
+		if err != nil {
+			fmt.Printf("Error writing response: %v\n", err)
 		}
-		return
-	}
 
-	w.Write([]byte(value))
+	default:
+		http.Error(w, "Method not allowed", http.StatusMethodNotAllowed)
+	}
 }
 
+// handleStore returns all non-reserved key-value pairs in the store by querying the database
+// GET /store
 func (hs *HTTPServer) handleStore(w http.ResponseWriter, r *http.Request) {
 	if r.Method != http.MethodGet {
 		http.Error(w, "Method not allowed", http.StatusMethodNotAllowed)
 		return
 	}
 
-	store := hs.executor.GetAllEntries()
+	store := make(map[string]string)
+	q := query.Query{} // Query all entries
+	results, err := hs.executor.db.Query(r.Context(), q)
+	if err != nil {
+		http.Error(w, "Failed to query store", http.StatusInternalServerError)
+		fmt.Printf("Error querying datastore: %v\n", err)
+		return
+	}
+	defer results.Close()
+
+	for result := range results.Next() {
+		if result.Error != nil {
+			http.Error(w, "Failed during store iteration", http.StatusInternalServerError)
+			fmt.Printf("Error iterating datastore results: %v\n", result.Error)
+			return
+		}
+		// Exclude reserved genesis keys from the output
+		dsKey := ds.NewKey(result.Key)
+		if dsKey.Equal(genesisInitializedKey) || dsKey.Equal(genesisStateRootKey) {
+			continue
+		}
+		store[result.Key] = string(result.Value)
+	}
 
 	w.Header().Set("Content-Type", "application/json")
 	if err := json.NewEncoder(w).Encode(store); err != nil {
@@ -218,5 +275,7 @@ func (hs *HTTPServer) handleStats(w http.ResponseWriter, r *http.Request) {
 	}
 
 	w.Header().Set("Content-Type", "application/json")
-	json.NewEncoder(w).Encode(stats)
+	if err := json.NewEncoder(w).Encode(stats); err != nil {
+		fmt.Printf("Error encoding stats response: %v\n", err)
+	}
 }

apps/testapp/kv/http_server_test.go

@@ -61,7 +61,9 @@ func TestHandleTx(t *testing.T) {
 				t.Errorf("expected body %q, got %q", tt.expectedBody, rr.Body.String())
 			}
 
+			// Verify the transaction was added to the channel if it was a valid POST
 			if tt.method == http.MethodPost && tt.expectedStatus == http.StatusAccepted {
+				// Allow a moment for the channel send to potentially complete
 				time.Sleep(10 * time.Millisecond)
 				ctx := context.Background()
 				retrievedTxs, err := exec.GetTxs(ctx)
@@ -74,6 +76,7 @@ func TestHandleTx(t *testing.T) {
 					t.Errorf("expected channel to contain %q, got %q", tt.body, string(retrievedTxs[0]))
 				}
 			} else if tt.method == http.MethodPost {
+				// If it was a POST but not accepted, ensure nothing ended up in the channel
 				ctx := context.Background()
 				retrievedTxs, err := exec.GetTxs(ctx)
 				if err != nil {
@@ -127,7 +130,9 @@ func TestHandleKV_Get(t *testing.T) {
 			exec := NewKVExecutor()
 			server := NewHTTPServer(exec, ":0")
 
+			// Set up initial data if needed
 			if tt.key != "" && tt.value != "" {
+				// Create and execute the transaction directly
 				tx := fmt.Appendf(nil, "%s=%s", tt.key, tt.value)
 				ctx := context.Background()
 				_, err := exec.ExecuteTxs(ctx, [][]byte{tx}, 1, time.Now(), []byte(""))
@@ -158,12 +163,15 @@ func TestHandleKV_Get(t *testing.T) {
 }
 
 func TestHTTPServerStartStop(t *testing.T) {
+	// Create a test server that listens on a random port
 	exec := NewKVExecutor()
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		// This is just a placeholder handler
 		w.WriteHeader(http.StatusOK)
 	}))
 	defer server.Close()
 
+	// Test the NewHTTPServer function
 	httpServer := NewHTTPServer(exec, server.URL)
 	if httpServer == nil {
 		t.Fatal("NewHTTPServer returned nil")
@@ -173,23 +181,33 @@ func TestHTTPServerStartStop(t *testing.T) {
 		t.Error("HTTPServer.executor not set correctly")
 	}
 
+	// Note: We don't test Start() and Stop() methods directly
+	// as they actually bind to ports, which can be problematic in unit tests.
+	// In a real test environment, you might want to use integration tests for these.
+
+	// Test with context (minimal test just to verify it compiles)
 	_, cancel := context.WithCancel(context.Background())
 	defer cancel()
 
+	// Just create a mock test to ensure the context parameter is accepted
+	// Don't actually start the server in the test
 	testServer := &HTTPServer{
 		server: &http.Server{
-			Addr:              ":0",
-			ReadHeaderTimeout: 10 * time.Second,
+			Addr:              ":0",             // Use a random port
+			ReadHeaderTimeout: 10 * time.Second, // Add timeout to prevent Slowloris attacks
 		},
 		executor: exec,
 	}
 
+	// Just verify the method signature works
 	_ = testServer.Start
 }
 
+// TestHTTPServerContextCancellation tests that the server shuts down properly when the context is cancelled
 func TestHTTPServerContextCancellation(t *testing.T) {
 	exec := NewKVExecutor()
 
+	// Use a random available port
 	listener, err := net.Listen("tcp", "127.0.0.1:0")
 	if err != nil {
 		t.Fatalf("Failed to find available port: %v", err)
@@ -202,15 +220,19 @@ func TestHTTPServerContextCancellation(t *testing.T) {
 	serverAddr := fmt.Sprintf("127.0.0.1:%d", port)
 	server := NewHTTPServer(exec, serverAddr)
 
+	// Create a context with cancel function
 	ctx, cancel := context.WithCancel(context.Background())
 
+	// Start the server
 	errCh := make(chan error, 1)
 	go func() {
 		errCh <- server.Start(ctx)
 	}()
 
+	// Give it time to start
 	time.Sleep(100 * time.Millisecond)
 
+	// Send a request to confirm it's running
 	client := &http.Client{Timeout: 1 * time.Second}
 	resp, err := client.Get(fmt.Sprintf("http://%s/store", serverAddr))
 	if err != nil {
@@ -224,8 +246,10 @@ func TestHTTPServerContextCancellation(t *testing.T) {
 		t.Fatalf("Expected status 200, got %d", resp.StatusCode)
 	}
 
+	// Cancel the context to shut down the server
 	cancel()
 
+	// Wait for shutdown to complete with timeout
 	select {
 	case err := <-errCh:
 		if err != nil && errors.Is(err, http.ErrServerClosed) {
@@ -235,6 +259,7 @@ func TestHTTPServerContextCancellation(t *testing.T) {
 		t.Fatal("Server shutdown timed out")
 	}
 
+	// Verify server is actually shutdown by attempting a new connection
 	_, err = client.Get(fmt.Sprintf("http://%s/store", serverAddr))
 	if err == nil {
 		t.Fatal("Expected connection error after shutdown, but got none")
@@ -245,6 +270,7 @@ func TestHTTPIntegration_GetKVWithMultipleHeights(t *testing.T) {
 	exec := NewKVExecutor()
 	ctx := context.Background()
 
+	// Execute transactions at different heights for the same key
 	txsHeight1 := [][]byte{[]byte("testkey=original_value")}
 	_, err := exec.ExecuteTxs(ctx, txsHeight1, 1, time.Now(), []byte(""))
 	if err != nil {
@@ -259,6 +285,7 @@ func TestHTTPIntegration_GetKVWithMultipleHeights(t *testing.T) {
 
 	server := NewHTTPServer(exec, ":0")
 
+	// Test GET request - should return the latest value
 	req := httptest.NewRequest(http.MethodGet, "/kv?key=testkey", nil)
 	rr := httptest.NewRecorder()