pilichoumao opened a new issue, #6278: URL: https://github.com/apache/rocketmq-dashboard/issues/6278
### Before Creating the Bug Report - [x] I searched open and historical issues and PRs; related contributions and the different failing order are listed below. - [x] This is a RocketMQ Studio defect, not a client usage question or another RocketMQ project. - [x] The service/DTO reproduction uses unchanged production classes from the revision below. ### Studio Version Default branch: `master@0228dad5b9c9460f3e18c5c3e2b56c6525856198`. Also reproduced on `rocketmq-studio@5e4c39b053c35a5598cc79ff331b8a54e649d7b1`. The existing local checkout also contains #6210, which changes only an unrelated MySQL upgrade script and its test. For the default-branch check, the master LlmConfigService was fetched at the exact SHA and compiled separately; SettingsService and GeneralSettingsUpdateDTO were verified byte-identical to the Studio snapshot. Existing compiled dependencies were reused. This is a targeted reproduction, not a full master build. ### Runtime Environment Java 21 on macOS, actual `SettingsService`, `LlmConfigService`, and `GeneralSettingsUpdateDTO`; mocked persistence returns independent snapshots. Controlled latches for the AI-old-snapshot case. This evidence is service/DTO level, not a live browser, HTTP, database transaction or multiple-server deployment test. ### Connected RocketMQ Cluster None required or contacted. No model provider, notification channel, or real credential was used. ### Describe the Bug General settings and AI configuration save the same complete persisted settings object. Each can carry non-null old fields owned by the other form. The fresh GET introduced for the general form narrows the interval but does not make the subsequent read/modify/write atomic. **Order A — AI overwrites a newer general change:** 1. `LlmConfigService.saveConfig` reads the general snapshot (`theme=light`). Pause that request after the snapshot is captured. 2. A general settings save completes with `theme=dark`. 3. Resume AI save. Its builder copies `current.getTheme()` from the old snapshot and `SettingsService.saveGeneralSettings` persists it. The final theme is `light`. `SettingsService.saveGeneralSettings` and `LlmConfigService.saveConfig` synchronize on different objects, and the AI snapshot construction happens outside the settings write monitor. **Order B — a general request overwrites a newer AI change:** 1. A successful fresh GET supplies `model=old-model` to the general form's `buildPayload`; construct its pending theme-change request from that snapshot. 2. AI save completes with `model=new-model`. 3. The pending general request completes. The actual `GeneralSettingsUpdateDTO.toSettings()` passes its non-null `old-model` to the settings service. The final model is `old-model`, while the requested theme change succeeds. The general DTO requires `llmProvider`, `model`, and `baseUrl`. Simply adding a shared Java lock to the AI save does not resolve a previously constructed, valid general request carrying stale AI fields. Silently ignoring those fields would also change the existing general endpoint's contract. ### Steps to Reproduce The controlled probe used the actual compiled services and DTO with a copying, in-memory mocked repository. It observed: ```text settings: after general save, theme=dark settings: after older LLM snapshot saves, theme=light reverse: after AI save, model=new-model reverse: after old general request, model=old-model, theme=dark ``` The first case pauses the actual repository load on the AI writer using two bounded CountDownLatches, completes the general save, then releases AI save. The second constructs a complete valid general DTO from the preceding read, completes AI save, then invokes `saveGeneralSettings(dto.toSettings())`. Neither changes production sources. Probe assertions verify the erroneous final values; exit code 0 means the existing overwrite was reproduced, not that a fix or a full test suite passed. Relevant source paths at the stated revision: - `server/src/main/java/org/apache/rocketmq/studio/ops/ai/LlmConfigService.java`: `saveConfig`. - `server/src/main/java/org/apache/rocketmq/studio/settings/SettingsService.java`: `saveGeneralSettings`. - `server/src/main/java/org/apache/rocketmq/studio/settings/GeneralSettingsUpdateDTO.java`: required AI fields and `toSettings`. - `web/src/pages/settings/GeneralSettingsTab.tsx`: `loadFreshSettings` and `buildPayload`. ### What Did You Expect to See? Successful edits of independent settings fields should both survive, or a conflicting stale request should be rejected visibly. Saving an AI-only form should preserve the latest non-AI values, and changing a general preference should not silently roll back an intervening AI save. ### What Did You See Instead? Both operations return normally at the service boundary, but an unrelated field returns to its earlier value. The two observed directions are theme `dark -> light` and model `new-model -> old-model`. ### Additional Context Related work and credit: - #4250 / #4255 reported the mount-time stale general snapshot; #5212 merged on rocketmq-studio refreshes before saving and aborts when that read fails. The current master still has the fallback-on-read-failure behavior, but this report deliberately uses a successful fresh read, so it concerns a separate remaining interleaving and the inverse server-side AI snapshot overwrite. - #2444 / #2450 proposed serialization of frontend form saves; #5384 serializes notification actions in one component. Independent requests or browser clients remain possible. - #2863 / merged #2963 preserve omitted fields and configuration/persistence consistency. Non-null stale values bypass omission backfill. - #5254 fixes the analogous whole-row overwrite for cloud credentials, which use different storage and requests. #5834 adds general DTO tests, not a concurrency fix. Searches covered `settings concurrent`, `settings race`, `LlmConfigService`, and the specific shared services. No open PR covering these two write orders was found at reporting time. **Design feedback requested before implementation:** should the console use explicit field-scoped updates while retaining the existing full general endpoint for compatibility, or should stale full-object saves carry a version/precondition and fail visibly? The solution must also keep omitted/explicitly cleared secrets, environment-token non-persistence, failed-save behavior and network-free critical sections intact. Multi-server safety requires a database-level consistency mechanism; a Java monitor alone must not be described as sufficient. I have not changed production code or submitted an implementation PR. Following CONTRIBUTING.md, I will wait for agreement on the write contract before adding an API or refactoring persistence. Real persistence/concurrency integration tests are still required for the chosen implementation. Please also confirm the preferred implementation base: the repository API reports master as default, while both branches' visible CONTRIBUTING.md still specify rocketmq-studio and recent fixes merged there. AI assistance was used for source review and the controlled service/DTO probes; the probes were executed and their original output inspected. ### Are You Willing to Submit a Pull Request? - [x] Yes, after agreement on the compatibility and concurrency scope. -- 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]
