Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add piping into zfs send|recv #761

Closed
wants to merge 2 commits into from
Closed

Conversation

dsh2dsh
Copy link
Contributor

@dsh2dsh dsh2dsh commented Oct 26, 2023

Use case is I'd like zfs send will actually do

zfs send | mbuffer -q -s 128k -m16M

and the same about zfs receive. Configuration looks like:

  - name: "remote-to-zdisk"
    type: "pull"
    connect:
      type: "tls"
      address: "remote:8888"
    recv:
      execpipe:
        # mbuffer | unzstd | zfs receive
        - [ "mbuffer", "-q", "-s", "128k", "-m", "16M" ]
        - [ "unzstd" ]

or

  - name: "zroot-to-remote"
    type: "push"
    connect:
      type: "tls"
      address: "remote:8888"
    send:
      execpipe:
        # zfs send | zstd -3 | mbuffer
        - [ "zstd", "-3" ]
        - [ "mbuffer", "-q", "-s", "128k", "-m", "16M" ]

Usually it executes standalone zfs send or zfs recv, but using execpipe we can configure it to send stdout of zfs send to another programm(s) or send stdout of another programm(s) to zfs recv.

This change adds new field execpipe into send and recv, which is seq of seq.

Use case is I'd like `zfs send` will actually do
```
zfs send | mbuffer -q -s 128k -m16M
```

and the same about `zfs receive`. Configuration looks like:
```
  - name: "remote-to-zdisk"
    type: "pull"
    connect:
      type: "tls"
      address: "remote:8888"
    recv:
      execpipe:
        # unzstd | mbuffer | zfs receive
        - [ "unzstd" ]
        - [ "mbuffer", "-q", "-s", "128k", "-m", "16M" ]
```

or
```
  - name: "zroot-to-remote"
    type: "push"
    connect:
      type: "tls"
      address: "remote:8888"
    send:
      execpipe:
        # zfs send | zstd -3 | mbuffer
        - [ "zstd", "-3" ]
        - [ "mbuffer", "-q", "-s", "128k", "-m", "16M" ]
```

Usually it executes standalone `zfs send` or `zfs recv`, but using `execpipe` we
can configure it to send stdout of `zfs send` to another programm(s) or send
stdout of another programm(s) to `zfs recv`.

This change adds new field `execpipe` into `send` and `recv`, which is seq of
seq.
@gdevenyi
Copy link

Not a comment on the the code per-se, but shouldn't the recieve pipe be "mbuffer | unzstd | zfs receive" ?

@dsh2dsh
Copy link
Contributor Author

dsh2dsh commented Oct 26, 2023

You are absolutely right. My fault. I guided by source code of syncoid and in comments it has

 '$compressargs{'decomcmd'} | $mbuffercmd | $recvcmd'

but in the code I see it actually uses mbuffer | decomcmd. Thank you. I'll change examples.

dsh2dsh added a commit to dsh2dsh/zrepl that referenced this pull request Oct 26, 2023
@problame
Copy link
Member

What are the performance improvements you're seeing with this?

mbuffer has been an early ask, but, I argued (and continue to argue) that

  1. zrepl does / can do buffering internally
  2. zrepl can do compression internally

@dsh2dsh
Copy link
Contributor Author

dsh2dsh commented Nov 2, 2023

What are the performance improvements you're seeing with this?

It doesn't matter. This change isn't about mbuffer. That was just an example. This change is about user controlled ability to use anything between zfs send and zfs recv for this user needs.

@problame problame closed this May 7, 2024
@gdevenyi
Copy link

gdevenyi commented Aug 9, 2024

Is there any justification for why this was closed unmerged?

This discussion #775

Notes that while it did not have a performance improvement, it did solve certain issues with the "burstyness" of the send/receive process.

@dsh2dsh
Copy link
Contributor Author

dsh2dsh commented Aug 9, 2024

Is there any justification for why this was closed unmerged?

JFYI, this and many other improvements implemented in my fork

https://github.com/dsh2dsh/zrepl

Feel free to use it.

@gdevenyi
Copy link

gdevenyi commented Aug 9, 2024

Thanks @dsh2dsh you should pipe up in #775 as the author may be interested in some of your improvements.

@dsh2dsh
Copy link
Contributor Author

dsh2dsh commented Aug 9, 2024

the author may be interested

Unfortunately I don't think so. All my PRs were closed, so I decided to develop an independent fork. I don't believe I'll upstream my changes into this repo.

@ext4xfs
Copy link

ext4xfs commented Oct 7, 2024

It would be nice if @problame would explain why none of these improvements were merged, all of these PRs seem quite good for the project.

@dsh2dsh dsh2dsh deleted the execpipe branch October 18, 2024 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants