fix(permalink): use a shared lock when re-reading a concurrently created permalink

[luizotavio32]

SUMMARY

After #45059, bursts of 3 or more identical permalink requests for the same dashboard can still return 500 on MySQL. Each request that loses the race already holds a shared lock on the duplicate row, then asks for an exclusive lock to re-read it. The losers end up waiting on each other and MySQL aborts one of them with a deadlock (1213 Deadlock found when trying to get lock). Two concurrent requests don't trigger it.

This change makes that re-read take a shared lock instead. Shared locks don't conflict with each other, so the deadlock goes away, and the re-read still sees the row the winning request just committed. The exclusive lock used by the distributed-lock release is unchanged.

TESTING INSTRUCTIONS

Unit tests added and updated.

ADDITIONAL INFORMATION

• Has associated issue:
• Required feature flags:
• Changes UI
• Includes DB Migration (follow approval process in SIP-59)
  • Migration is atomic, supports rollback & is backwards-compatible
  • Confirm DB migration upgrade and downgrade tested
  • Runtime estimates and downtime expectations provided
• Introduces new feature or API
• Removes existing feature or API

🤖 Generated with Claude Code

[netlify]

✅ Deploy Preview for superset-docs-preview ready!

Name	Link
🔨 Latest commit	e3e60fc
🔍 Latest deploy log	https://app.netlify.com/projects/superset-docs-preview/deploys/6ac9410567d5080008d2df4e
😎 Deploy Preview	https://deploy-preview-45102--superset-docs-preview.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.83%. Comparing base (a8fc576) to head (e3e60fc).

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #45102   +/-   ##
=======================================
  Coverage   82.83%   82.83%           
=======================================
  Files        3017     3017           
  Lines      192698   192709   +11     
  Branches    44914    44914           
=======================================
+ Hits       159624   159635   +11     
  Misses      30040    30040           
  Partials     3034     3034           

Flag	Coverage Δ
hive	35.35% <70.58%> (+<0.01%)	⬆️
mysql	53.80% <94.11%> (+<0.01%)	⬆️
postgres	53.81% <94.11%> (+<0.01%)	⬆️
presto	37.19% <70.58%> (+<0.01%)	⬆️
python	86.47% <100.00%> (+<0.01%)	⬆️
sqlite	53.55% <94.11%> (+<0.01%)	⬆️
unit	80.11% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[luizotavio32]

/review

[codeant-ai-for-open-source]

CodeAnt PR Risk: Low Risk

• The PR appears safe to merge; permalink duplicate-key retries now use shared locks to avoid deadlocks between concurrent losers on InnoDB.
• Distributed-lock release retains an exclusive lock for the ownership check and delete, and tests verify the shared-lock SQL for MySQL and PostgreSQL.

Assessed commit: ea8bb63626ce

[michael-s-molina]

Thanks for tracking this down. The shared-lock re-read makes sense: a locking read still returns the winner's committed row under REPEATABLE READ, and shared locks don't conflict with each other, so the 3+ loser deadlock goes away.

Suggestion: replace the booleans with a lock-options object

get_entry now has two mutually exclusive flags, for_update and for_share. Nothing enforces that. With if/elif, passing both silently ignores for_share. Every new lock mode (nowait, skip_locked, ...) would also need another boolean. Query.with_for_update already takes several arguments, so I'd model them directly:

@dataclass(frozen=True)
class RowLock:
    read: bool = False         # shared lock (FOR SHARE / LOCK IN SHARE MODE)
    nowait: bool = False
    skip_locked: bool = False
    key_share: bool = False    # Postgres only

    def apply(self, query: Query) -> Query:
        return query.with_for_update(
            read=self.read,
            nowait=self.nowait,
            skip_locked=self.skip_locked,
            key_share=self.key_share,
        )

def get_entry(cls, resource, key, lock: RowLock | None = None) -> KeyValueEntry | None:
    query = db.session.query(KeyValueEntry).filter_by(**get_filter(resource, key))
    if lock is not None:
        query = lock.apply(query)
    return query.first()