[yangjj-iso]

Fixes #5550.

Concurrent Admins could both observe a missing selector and insert duplicates during metadata registration. Hold the persistent plugin row lock and perform a current locking selector lookup inside one transaction before creation. The parent lock serializes the missing-row case across processes; namespace remains part of selector identity.

Validation: two independent service proxies use real H2/MyBatis connections for eight concurrent registration pairs, asserting the same returned ID and exactly one row per path. All 18 selector service/concurrency tests pass on Microsoft OpenJDK 17.0.18. Compilation, Checkstyle, Apache RAT and git diff --check pass. MySQL/PostgreSQL/Oracle and Docker cluster E2E were not run.

Review follow-up: default registration and the existing locking lookup now share one implementation, including a current selector read under the plugin lock. The mapper explains the REPEATABLE READ reason; unknown plugins have an explicit diagnostic. The per-plugin lock covers creation and notifications until commit. All 18 selector service/concurrency tests pass, including sequential lookup of an already registered selector. MySQL/PostgreSQL/Oracle concurrency is still not claimed.

Shared CI fixture corrections: CryptorResponsePluginTest now checks the post-#7903 configuration error for both encrypt/decrypt when a field is absent, including absence of the original user data. SDK tests wait up to 60 seconds for usable registered routes before exercising assertions. ZooKeeper registration tests keep their declared backend ports reachable so the real health checker cannot race registry-removal assertions. Validation: 16 cryptor unit cases and 16 Admin discovery/health cases pass; the HTTP and SDK integration test reactors compile with Checkstyle. Full Docker integration execution is not claimed: the local Linux Docker engine is unavailable. Remote check results are tracked separately.

[Aias00]

Review: serialize default selector registration across Admins

The approach is right. SELECT ... FOR UPDATE on a row that does not exist yet locks nothing, so locking the selector is not enough; taking the persistent plugin row first is the correct way to serialize the missing-row case across Admin instances. I verified the pieces:

• PluginMapper.selectByNameForUpdate already exists and its SQL is WHERE name = #{name} FOR UPDATE (plugin-sqlmap.xml:73), so no new plugin-side mapper is needed.
• The H2 schema seeds the divide plugin (sql-script/h2/schema.sql:568), so selectByNameForUpdate("divide") returns a row in the new test instead of NPE-ing — that was the first thing I checked, since the whole design hinges on a parent row that always exists.
• @Transactional on the (MetaDataRegisterDTO, String, String) overload is the correct placement: registerDefault(SelectorDTO) is invoked by self-invocation two lines below, so it joins the outer transaction and the plugin lock is held until the insert commits. Putting the annotation only on the inner overload would have left FOR UPDATE running in autocommit and the fix would have been a silent no-op. All callers (AbstractShenyuClientRegisterServiceImpl, AbstractContextPathRegisterService) go through the injected SelectorService proxy, so the annotation actually applies.
• The new test is a genuine regression test: with the pre-fix code both threads read null and both insert, so the COUNT(*) = 1 assertion fails. Two independently proxied service instances sharing one H2 datasource and one DataSourceTransactionManager is a fair stand-in for two Admin processes. Constructor argument order matches SelectorServiceImpl exactly.

Non-blocking notes:

1. Duplication. The service already exposes findByNameAndPluginNameAndNamespaceIdForUpdate(name, pluginName, namespaceId) (line 428), which takes the same plugin lock but reads the selector with the non-locking mapper method. registerDefault now re-implements that sequence inline with a locking selector read. Two near-identical paths with different locking semantics is a trap for the next reader — either upgrade the existing helper to the locking read and call it here, or document why the two differ.
2. The selector FOR UPDATE is redundant and untested. Once the plugin row is held, the check-then-insert is already serialized, so the second lock buys nothing in the missing-row case; the new test passes with the plugin lock alone. If the intent is to force a current read regardless of isolation level (REPEATABLE READ snapshot staleness), say so in the mapper Javadoc, because right now it reads like an oversight. Dropping it would keep the lock footprint at exactly one row.
3. Lock scope now spans the whole create-and-notify sequence. Because registerDefault(SelectorDTO) joins the new transaction, selectorEventPublisher.publish(...) / publishEvent(...) run while the plugin row lock is held, so every concurrent registration for the same plugin serializes for the full default-selector + default-rule creation and gateway notification. That matches the existing createOrUpdate convention in this class, so it is not a regression — but the plugin row is now a per-plugin global serialization point, which deserves a line in the Javadoc.
4. pluginDO can still be null for an unknown pluginName, and the NPE now happens one statement earlier than before. Objects.requireNonNull(pluginDO, "unknown plugin: " + pluginName) would turn a bare NPE into something diagnosable.
5. The losing thread in each iteration does exercise the "already exists" branch (it returns the winner's id without inserting), which is good. What is still uncovered is the plain steady-state path — a single registration against an existing selector with no contention — though that is a cheap addition rather than a gap in this fix.
6. MySQL/PostgreSQL/Oracle behaviour is unverified (as the PR body states). H2's FOR UPDATE on the plugin row is a reasonable proxy, but gap-lock behaviour on InnoDB under REPEATABLE READ is exactly the kind of thing that differs — worth a follow-up note on the issue rather than a blocker here.

LGTM once the duplication in (1) is at least acknowledged.

[Aias00]

Re-review after 1f17827e: the lock now spans creation and notification, which is what was missing

The previous round left the shared lockSelector refactor optional; the author did it, and it is the right shape:

• registerDefault(dto, pluginName, selectorHandler) is now @Transactional and takes pluginMapper.selectByNameForUpdate(pluginName) before looking the selector up. The parent plugin row exists in every case, so the lock works even when no selector exists yet - which is exactly the case the old findByNameAndPluginNameAndNamespaceId read could not serialise.
• findByNameAndPluginIdAndNamespaceIdForUpdate is a SELECT ... FOR UPDATE current read, so two Admins on REPEATABLE READ cannot both see "no selector" and both insert. Both entry points now share lockSelector, so the API can no longer be called in the unlocked variant by accident.
• The javadoc on SelectorService.findByNameAndPluginNameAndNamespaceIdForUpdate now states that it requires a caller-owned transaction, which is the contract that makes the lock meaningful.

Test quality is the notable part: SelectorRegistrationConcurrencyTest boots a real H2 (MySQL mode), loads the actual plugin-sqlmap.xml / selector-sqlmap.xml, wraps two independent SelectorServiceImpl instances in a ProxyFactory with a real TransactionInterceptor, then races them through a CyclicBarrier over 8 paths and asserts both the returned id and COUNT(*) = 1. Using the real transaction advice is what makes the test able to fail - a plain unit test with mocks would have passed before and after.

Checked and fine:

• SelectorServiceTest.testHandlerSelectorNeedUpstreamCheck was updated to stub selectByNameForUpdate, consistent with the new path.
• The inner registerDefault(selectorDTO) call keeps running inside the outer transaction (self-invocation on the same instance), so the selector insert and the event publication stay under the plugin lock until commit.

Nits, non-blocking:

1. Objects.requireNonNull(pluginMapper.selectByNameForUpdate(pluginName), "unknown plugin: " + pluginName) fails with a bare NullPointerException. The message is good, but ShenyuException (or IllegalArgumentException) would be the honest type for "plugin does not exist", and the previous code NPE'd only incidentally.
2. This branch carries test(ci): align cryptor failures and wait for usable SDK routes plus the ZookeeperClientDiscoveryIntegrationTest ServerSocket change. Those belong to #7970 / the shared CI fix, not to this PR. Please rebase once #7970 lands so the same commit is not merged four times (this PR, #7867, #7868, #7948) and does not conflict on the way in.