[jira] [Resolved] (YUNIKORN-3348) Develop doc fixups

["Manikandan R (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/YUNIKORN-3348?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Manikandan R resolved YUNIKORN-3348.
------------------------------------
     Fix Version/s: 1.10.0
    Target Version: 1.10.0
        Resolution: Fixed

> Develop doc fixups
> ------------------
>
>                 Key: YUNIKORN-3348
>                 URL: https://issues.apache.org/jira/browse/YUNIKORN-3348
>             Project: Apache YuniKorn
>          Issue Type: Improvement
>          Components: documentation
>            Reporter: Dale Richardson
>            Assignee: Dale Richardson
>            Priority: Major
>              Labels: pull-request-available
>             Fix For: 1.10.0
>
>
> The only statement of the scheduler's lock-ordering rule anywhere on the site is malformed, and the surrounding text has been overtaken by a feature added in 2024. Two adjacent gaps make the same information hard to find. All four items below are small, self-contained fixes.
> h3. 1. The lock-order rule is garbled
> {{docs/design/cache_removal.md}}, lines 424-426:
> {quote}
> It is possible to acquire another lock while holding a lock, but we need to make sure that we do not allow:
> * Holding A.lock and acquire B's lock.
> * Holding B.lock and acquire B's lock.
> {quote}
> The second bullet should read "Holding *B.lock* and acquire *A's* lock". As written it says "B then B", which is a tautology, so the rule as published states nothing.
> This is the only lock-ordering statement in the documentation. The same text appears in every versioned copy from 1.3.0 through 1.9.0.
> Suggested fix:
> {noformat}
> - Holding A.lock and acquire B's lock.
> - Holding B.lock and acquire A's lock.
> {noformat}
> h3. 2. "No known tools" is no longer true
> {{docs/design/cache_removal.md}}, line 456:
> {quote}
> There are no known tools that could be used to detect or describe lock order.
> {quote}
> This has been false since YUNIKORN-2539 (commit 5758d7a, 2024-04-05) added {{pkg/locking}}, which wraps {{github.com/sasha-s/go-deadlock}} and performs both lock-order (ABBA) detection and lock-wait timeout detection.
> Suggested replacement:
> {quote}
> Lock order and lock-wait timeouts can be detected at runtime. The {{pkg/locking}} package wraps {{go-deadlock}} and is enabled via the environment variables described in the service configuration documentation.
> {quote}
> h3. 3. Deadlock detection settings are undocumented
> {{pkg/locking}} is configured entirely by environment variables, none of which appear anywhere on the site:
> || Variable || Default || Effect ||
> | {{DEADLOCK_DETECTION_ENABLED}} | false | Master switch. go-deadlock is compiled in either way, it is only disabled. |
> | {{DEADLOCK_TIMEOUT_SECONDS}} | 60 | Lock-wait threshold before a potential deadlock is reported. |
> | {{DEADLOCK_EXIT}} | false | Call {{os.Exit(1)}} when a potential deadlock is detected. |
> | {{DEADLOCK_DISABLE_LOCK_ORDER}} | false | Disable ABBA lock-order detection, keeping only the timeout check. |
> {{docs/user_guide/service_config.md}} already documents comparable runtime knobs ({{GOGC}}, {{GOMEMLIMIT}}), so that is the natural home.
> This matters operationally: with {{DEADLOCK_EXIT=true}} the scheduler process terminates on detection, and an operator currently has nothing to look up. The detection state is also exposed over REST — {{/ws/v1/config}} returns {{DeadlockDetectionEnabled}} and {{DeadlockTimeoutSeconds}} — with no documentation of what those fields mean.
> h3. 4. {{make test}} runs with detection enabled, which is not stated
> {{docs/developer_guide/build.md}}, line 165, says only:
> {quote}
> * A full unit test run {{make test}}
> {quote}
> The {{test}} target in both yunikorn-core and yunikorn-k8shim exports {{DEADLOCK_DETECTION_ENABLED=true}}, {{DEADLOCK_TIMEOUT_SECONDS=10}} and {{DEADLOCK_EXIT=true}}, and runs the suite under {{-race}}. A contributor whose test run exits with a "POTENTIAL DEADLOCK" dump has no documentation explaining what produced it or how to adjust the timeout.
> h3. Proposed changes
> # Fix the A/B typo at {{cache_removal.md}} lines 425-426.
> # Replace the "no known tools" sentence at {{cache_removal.md}} line 456 with a reference to {{pkg/locking}} and the variables above.
> # Add a "Deadlock detection" subsection to {{docs/user_guide/service_config.md}} documenting the four environment variables.
> # Extend the {{make test}} bullet in {{docs/developer_guide/build.md}} to state that it runs with {{-race}} and deadlock detection enabled (exit on detection, 10 second timeout).
> h3. Note on cache_removal.md
> {{cache_removal.md}} is a design proposal for a migration completed under YUNIKORN-317, yet its "Current locking" section is the de facto locking reference for the project. The project already has a convention for this situation: documents under {{docs/archived_design/}} carry a {{:::caution}} banner naming the superseding document. Either the locking content should be lifted into a maintained developer guide page, or the document should state plainly which parts are still current. A follow-up issue is proposed for that.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@yunikorn.apache.org
For additional commands, e-mail: dev-help@yunikorn.apache.org