Akanksha-kedia commented on PR #18933: URL: https://github.com/apache/pinot/pull/18933#issuecomment-4957332063
@xiangfu0 Thank you for the thorough review. Addressed all three new comments in the latest commit: | Comment | Fix | |---|---| | **Cross-controller session store** (`BaseControllerStarter.java:758`) | Added a comment block documenting that `InMemorySessionManager` is per-process and that sticky sessions are required for reliable cross-controller logout in LB deployments. Added detail on the degraded-but-functional behavior without sticky routing. A shared store (Redis/ZK) would be a separate follow-up PR. | | **Cookie Max-Age / sliding TTL mismatch** (`PinotSessionLoginResource.java:281`) | Removed the sliding-window TTL extension from `InMemorySessionManager.getUsername()`. Both server session and browser cookie now expire at `loginTime + sessionTtlSeconds`. The `getUsername()` implementation is now a simple get+check (no `compute()` mutation), matching the existing `getBasicAuthToken()` semantics. Updated `SessionManagerTest` to verify fixed TTL behavior (calling `getUsername` mid-session does not extend expiry). | | **CORS allowed headers missing `database`** (`ControllerAdminApiApplication.java:155`) | Added `Database` and `If-Match` to `Access-Control-Allow-Headers`. `Database` is the custom Pinot header used by `DatabaseUtils.translateTableName` for multi-database resolution; browser preflight was rejecting requests that set it. | -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
