yuqi1129 opened a new pull request, #13048:
URL: https://github.com/apache/gravitino/pull/13048

   ### What changes were proposed in this pull request?
   
   Two changes that let an integration suite know which server it is talking to.
   
   - `BaseIT` refuses to launch the deploy mode server when something already 
listens on the configured port, and names the port in the message. 
`ITUtils.checkServerPortIsFree` does the check.
   - `MiniGravitinoContext` carries the server to start, defaulting to 
`GravitinoServer::main`, and `MiniGravitino` starts that one. Existing callers 
are unaffected.
   
   ### Why are the changes needed?
   
   **Any server on the port is accepted as ours.** `BaseIT.startServer()` 
decides the deploy mode server is ready by polling `/metrics` until an HTTP 
server answers, and never checks that the process answering is the one it just 
launched. A Gravitino left behind by an interrupted run satisfies the probe, so 
the suite runs against a stranger with unrelated configuration.
   
   This is easy to hit, because `gravitino.sh` fails to launch whenever a port 
it needs is taken, including the debugger port, and the leftover holding that 
port is usually also holding the server port. In my case the launch failed four 
times:
   
   ```
   ERROR: transport error 202: bind failed: Address already in use
   ERROR: JDWP Transport dt_socket failed to initialize, TRANSPORT_INIT(510)
   ```
   
   and the suite then ran against a leftover server that had authorization 
enabled, so `createMetalake` came back as:
   
   ```
   ForbiddenException: Forbidden error :User 'anonymous' is not authorized to 
perform
   operation 'createMetalake' : Only service admins can create metalakes
   ```
   
   even though the configuration under test had `gravitino.authorization.enable 
= false` and `gravitino.authorization.serviceAdmins = anonymous`. The failure 
points at authorization, and the cause is a stale process; it took several 
rounds to find that in `logs/gravitino-server.out`.
   
   **Embedded mode cannot start a downstream server.** `MiniGravitino` 
hardcodes `GravitinoServer.main`, so a distribution that ships its own entry 
point can only run integration tests in deploy mode. The failure mode is quiet 
rather than loud: in embedded mode the components its server would initialize 
are simply absent, so the event listeners that depend on them throw on every 
event and the framework logs and swallows it:
   
   ```
   WARN  ...Listener - Failed to handle event RegisterJobTemplateEvent@201b3159
   java.lang.NullPointerException: ... because "this.searchService" is null
   ```
   
   The suite keeps running, nothing is ever written, and assertions that 
tolerate an empty result pass. A test can report success while exercising 
nothing.
   
   Fix: #13044
   Fix: #13045
   
   ### Does this PR introduce _any_ user-facing change?
   
   No. Both changes are in the integration test harness.
   
   For test authors: deploy mode now fails fast, with the port named and an 
`lsof` invocation to find the holder, instead of running against whatever 
answers. `MiniGravitinoContext` gains a constructor that takes a 
`ServerLauncher`; the existing constructor keeps starting `GravitinoServer`.
   
   ### How was this patch tested?
   
   - `TestITUtils` covers both sides of the port check: a free port passes, and 
a port held by an open `ServerSocket` raises `IllegalStateException` whose 
message names the port.
   - `TestMiniGravitino` covers that the default context launches 
`GravitinoServer` and that a custom launcher is the one invoked.
   - `./gradlew :integration-test-common:test -PskipITs`
   
   Both tests were written first and failed to compile against the old API.
   
   Worth a reviewer's opinion: `EventListenerPluginWrapper` logs and swallows 
every listener failure, which is what turned the second problem into a silent 
one. A listener failing on every event is closer to a broken deployment than to 
a recoverable error. I left that alone here, but it seems worth addressing 
separately.
   


-- 
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]

Reply via email to