tigerquoll opened a new pull request, #571:
URL: https://github.com/apache/yunikorn-site/pull/571

   ### Description
   
   The only statement of the scheduler's lock ordering rule anywhere on the 
site was malformed, and the text around it predates the deadlock detection 
feature added in YUNIKORN-2539.
   
   - **`docs/design/cache_removal.md`** — correct the lock direction rule. The 
second bullet read *"Holding B.lock and acquire B's lock"*, which is a 
tautology, so the rule as published stated nothing. It should be *"acquire A's 
lock"*. This is the only lock-ordering statement in the documentation.
   - **`docs/design/cache_removal.md`** — replace *"There are no known tools 
that could be used to detect or describe lock order"* with a reference to 
`pkg/locking`. That has been inaccurate since YUNIKORN-2539 (commit `5758d7a`, 
2024-04-05) added the package, which wraps 
[go-deadlock](https://github.com/sasha-s/go-deadlock) and performs exactly lock 
order and lock wait detection.
   - **`docs/user_guide/troubleshooting.md`** — new "Deadlock detection" 
section documenting the four `DEADLOCK_*` environment variables, the two checks 
they control, where the report is logged (`core.diagnostics`, `ERROR`), and how 
to read the current setting back from `/ws/v1/config`.
   - **`docs/developer_guide/build.md`** — state that `make test` runs with 
`-race` and deadlock detection enabled, so a run ending in `POTENTIAL DEADLOCK` 
is a real lock problem rather than a flaky test.
   
   #### Two notes for reviewers
   
   **Placement.** The Jira suggests `service_config.md` for the environment 
variables. I have put them in `troubleshooting.md` instead: `service_config.md` 
documents Helm values and ConfigMap keys, and these are neither — they are 
environment variables set on the container. `troubleshooting.md` already covers 
log retrieval, the state dump and restarting the scheduler, which is the 
context an operator is in when they need this. Happy to move it if you would 
prefer to keep all runtime settings together.
   
   **Versioned docs.** This changes `docs/` only, matching recent documentation 
PRs (#563, #564, #565), so the corrections appear in the next release. The 
published 1.9.0 docs keep the garbled rule until then. Let me know if you would 
like the two `cache_removal.md` corrections backported to 
`versioned_docs/version-1.9.0/` as well — they are factual errors rather than 
new content, so there is an argument for it.
   
   ### Type of change
   
   - [x] Documentation
   - [x] Bug Fix
   
   ### Jira issue
   
   Jira ID : https://issues.apache.org/jira/browse/YUNIKORN-3348
   
   - [x] I have created a Jira issue for this pull request
   - [x] The Jira ID is part of the title of this pull request
   
   ### AI Tooling
   
   Generated by the Author with assistance from Claude Code.
   
   - [x] The PR includes the phrase "Generated by \<tool>", where \<tool> is 
the name of the AI tool used.
   - [x] My use of AI contributions follows the ASF legal policy.
   
   Check https://www.apache.org/legal/generative-tooling.html for details.
   
   ### How Has This Been Tested?
   
   - [x] `check_license.sh` returned an "all OK" result
   - [ ] `local_build.sh run` generated a usable website
   
   Instead of the Docker wrapper, the site was built directly with `pnpm 
install --frozen-lockfile && pnpm build`, which runs the same Docusaurus build. 
Result: `[SUCCESS] Generated static files in "build"`.
   
   Checks performed on the generated output:
   
   - The new `## Deadlock detection` heading produces the anchor 
`#deadlock-detection` in 
`build/docs/next/user_guide/troubleshooting/index.html`.
   - Both new cross-document links resolve, appearing in the generated HTML as 
`href="/docs/next/user_guide/troubleshooting#deadlock-detection"` from 
`design/cache_removal` and `developer_guide/build`.
   - The build reports pre-existing broken-anchor warnings elsewhere on the 
site; none of them are on the three pages touched here, and this change adds 
none.
   
   ### Screenshots or other details
   
   Sources for the factual claims in the new documentation, so they can be 
checked:
   
   - The four environment variables, their defaults and behaviour: 
`yunikorn-core/pkg/locking/locking.go`.
   - The shim delegates its configuration to the core, so one set of settings 
covers both: `yunikorn-k8shim/pkg/locking/locking.go` calls into `corelocking` 
from its `init()`.
   - Reports are logged under `core.diagnostics` at `ERROR` level: 
`printBufContents()` in the same core file.
   - `/ws/v1/config` returns `DeadlockDetectionEnabled` and 
`DeadlockTimeoutSeconds`: `yunikorn-core/pkg/webservice/dao/config_info.go` and 
`handlers.go`.
   - `make test` exports `DEADLOCK_DETECTION_ENABLED=true`, 
`DEADLOCK_TIMEOUT_SECONDS=10`, `DEADLOCK_EXIT=true` and runs with `-race` in 
both repositories: the `test` target in each `Makefile`.
   


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