k0sproject / k0sproject/rig

remotefs/winfile: rigrcp writes cost ~5x more WinRM round trips than needed, and what rigrcp is actually for

Open
#475 0 comments 0 reactions 0 assignees View on GitHub
enhancement
Dominant language
Go
Stars
53
Forks
33
Avg merge
1d 9h
Merged PRs (30d)
29

Description

## Why rigrcp exists

Worth writing down, because it decides what can be removed. Two reasons, different in kind:

1. **`fs.File` needs a stateful handle.** The origin commit is *"Implement fs.FS / fs.File for connections"* (#81). `Read`/`Write`/`Seek` with a position implies something on the far end holding an open handle. The POSIX side solves the same problem *statelessly* — `PosixFile.Read` shells out to `dd skip=… count=…` per call and tracks `pos` in Go. That works because an SSH exec is cheap. Over WinRM every command is `CreateShell` → `ExecuteWithContext` → output polling → `Close`, several authenticated HTTP round trips. Per chunk that is unaffordable, so the daemon amortizes shell creation across the whole file.

2. **Binary-safe stdio out of PowerShell.** The `GetStdHandle(-10)/(-11)` + `Add-Type` kernel32 block at the top of `rigrcp.ps1` is how you get raw byte streams instead of PowerShell's text-mangling defaults. *Any* streaming approach needs that, rigrcp or not.

(2) is the part worth remembering: the protocol is removable, the raw-handle plumbing is not.

## The actual cost is the write path

`payloadChunkSize` is 32 KiB, so `winFile.Write` does one `w N` round trip per 32 KiB. But `masterzen/winrm`'s `DefaultParameters` sets `EnvelopeSize` to 153600, and `commandWriter.Write` sends `min(EnvelopeSize-1000, len(data))` per SOAP request — so a 32 KiB chunk under-fills the envelope by about 4.7x.

For a 100 MB upload, by arithmetic (not measurement):

| | today, 32 KiB chunks | 4 MiB chunks |
|---|---|---|
| `w N` command + response round trips | ~3200 | ~25 |
| `sendInput` SOAP requests | ~3200 | ~690 |

The `sendInput` count is floored at ~690 by the envelope size; today we pay 3200 because the chunks are too small to fill an envelope, *and* a full command/response round trip on every one of them.

Reads are already fine: `r -1` does `$f.CopyTo($out)` — one command, one stream.

## Proposal A: raise the chunk (no API change)

Raise `payloadChunkSize` to something on the order of a few MiB. It applies to both write paths — `WinFS.WriteFile` (where `bytes.Reader` implements `io.WriterTo`, so `Write` already receives the whole payload and the chunk loop splits it back down) and `Upload`/`CopyFrom` (where `io.Copy` hands over 32 KiB at a time; that one would want `io.CopyBuffer` with a matching buffer, since `io.TeeReader` hides the source size from `CopyFrom`).

**This only became possible in #473.** Before it, `w N` did a single `$in.Read` and treated anything short as fatal, so a payload larger than one transport chunk was guaranteed to fail — the protocol could only ever be driven in transport-sized pieces. The accumulate loop is what unlocks the efficient shape.

**Interaction to handle:** `commandTimeout` is an idle timeout whose write-side unit of progress is one chunk (documented on `payloadChunkSize`). Raising the chunk raises the minimum rate a write must sustain — at 4 MiB per 30s that is ~140 KB/s, versus ~1 KB/s today. That is a real regression for slow links unless progress is reported at a finer grain.

The fix is available: `open()` hands `stdinR` to `f.fs.Start(ctx, rigRcp, cmd.Stdin(stdinR), …)`, and the `io.Copy` draining that pipe reads 32 KiB at a time, only reading again once the previous `sendInput` has completed. Wrapping that reader so each read reports progress to the file's active watchdog gives per-32-KiB progress *regardless* of chunk size — read N+1 happening is proof that send N landed. That decouples timeout granularity from protocol chunk size, which is the right split anyway.

## Proposal B: could rigrcp go away entirely?

The capability it uniquely provides — a *seekable* remote handle — appears to be used by nothing. Grepping rig, k0sctl and launchpad, the only `.Seek(` hit anywhere is inside a PowerShell string literal in `powershell/powershell.go`. k0sctl's two remote `OpenFile` calls are both `O_CREATE|O_WRONLY`, pure streaming writes.

So in principle: drop `Seek` from `remotefs.File` on Windows (deliberate and flagged; allowed within v2.x) and replace the daemon with two one-shot streaming commands. `fs.File` itself only requires `Stat`/`Read`/`Close`, so stdlib compliance survives — `Seek` is rig's own addition.

Before anyone reaches for that, the things that would have to be re-implemented rather than deleted:

- `Invoke-WithRetry … "*used by another process*"` — 10 retries against AV/indexer file locks, genuinely Windows-specific behaviour
- the open mode/access matrix (`CreateNew`/`OpenOrCreate`/`Truncate`/`Append` × `Read`/`Write`/`ReadWrite`)
- path rooting and `Resolve-Path` handling
- the whole `GetStdHandle` raw-stream setup, unchanged

## Suggested order

Proposal A first: it is contained, needs no API change, and takes the cost out of the path that actually gets used. Proposal B is worth deciding on eventually, but the ~4.7x is in A, not B.

Everything above is from reading the code and the `masterzen/winrm` constants, not from measurement. The `windows` integration job is the place to time an upload before and after if the numbers need substantiating first.

Contributor guide

No contributing guide indexed for this repository

Research direction

Read payloadChunkSize and commandTimeout handling first, then trace WinFS.WriteFile and Upload/CopyFrom through commandWriter and open(). Use the windows integration job to establish upload timing before and after. Done means Proposal A reduces unnecessary round trips while retaining fine-grained write progress; leave Proposal B as a separate design decision.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
backend, networking, performance
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.