Change tokio feature

[liurenjie1024]

What changes are included in this PR?

Changes tokio dependency, enable sync feature only by default, but requires full feature when we enable tokio cfg. With this change, we can still use tokio's runtime indepdenent utils, while only rely on tokio's runtime when we enable tokio cfg.

Are these changes tested?

CI.

[Copilot]

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (2)

crates/iceberg/Cargo.toml:41

• Verify that switching the dependency coordinate from "dep:tokio" to "tokio/full" correctly reflects the intended feature usage and version requirements. Ensure that the full feature is enabled only for the tokio runtime scenario as expected.

tokio = ["tokio/full"]

crates/iceberg/Cargo.toml:83

• Changing the optional flag to false may force inclusion of the tokio dependency in all environments. Confirm that this change aligns with the design of enabling tokio's runtime only when the tokio cfg is active and does not adversely affect builds that only use util functions.

tokio = { workspace = true, optional = false, features = ["sync"] }

[Xuanwo]

tokio/full is really big. Do we really want it?

[fqaiser94]

@Xuanwo for context, I believe this thread is at least part of the reason why @liurenjie1024 is proposing this change.

[Xuanwo]

I suggest we wait a bit to see how #1036 progresses. Ideally, all dependencies on cfg(tokio) can be eliminated once our mini engine has been extracted.

[liurenjie1024]

I suggest we wait a bit to see how #1036 progresses. Ideally, all dependencies on cfg(tokio) can be eliminated once our mini engine has been extracted.

@Xuanwo Seems we only need to enable rt-multithread, WDYT if I change full to rt-multithread? This is blocking #1165 , so I want to have it merged first. This pr also aligns with the goal of removing unnecessary cfg(tokio) , more details could be found #1165 (comment)

[Xuanwo]

WDYT if I change full to rt-multithread

Makes more sense to me.

[xxchan]

I'm a little confused here: If previously we didn't enable tokio/full, we won't need it also after changing optional from false to true.

Therefore, it's possible that some other dependency already enabled tokio/rt, even when we didn't enable that.

[xxchan]

This is the dependency tree of tokio:

cargo tree -i tokio -p iceberg -e features

tokio v1.44.2
└── hyper-util v0.1.10
    ├── hyper-util feature "client"
    │   └── reqwest v0.12.12
    │       └── reqsign v0.16.1
    │           └── opendal v0.51.2
    │               ├── opendal feature "default"
    │               │   └── iceberg v0.4.0 (/Users/xxchan/Projects/iceberg-rust/crates/iceberg)
    │               │       ├── iceberg feature "default" (command-line)
    │               │       │   └── iceberg-catalog-memory v0.4.0 (/Users/xxchan/Projects/iceberg-rust/crates/catalog/memory)
    │               │       │       └── iceberg-catalog-memory feature "default"
    │               │       │           [dev-dependencies]
    │               │       │           └── iceberg v0.4.0 (/Users/xxchan/Projects/iceberg-rust/crates/iceberg) (*)
    │               │       ├── iceberg feature "storage-fs"
    │               │       │   └── iceberg feature "default" (command-line) (*)
    │               │       ├── iceberg feature "storage-memory"
    │               │       │   └── iceberg feature "default" (command-line) (*)
    │               │       ├── iceberg feature "storage-s3"
    │               │       │   └── iceberg feature "default" (command-line) (*)
    │               │       └── iceberg feature "tokio"
    │               │           └── iceberg feature "default" (command-line) (*)
    │               ├── opendal feature "executors-tokio"
    │               │   └── opendal feature "default" (*)
    │               ├── opendal feature "internal-tokio-rt"
    │               │   └── opendal feature "services-fs"
    │               │       └── iceberg feature "storage-fs" (*)
    │               ├── opendal feature "services-fs" (*)
    │               ├── opendal feature "services-memory"
    │               │   ├── iceberg feature "storage-memory" (*)
    │               │   └── opendal feature "default" (*)
    │               └── opendal feature "services-s3"
    │                   └── iceberg feature "storage-s3" (*)
    │           ├── reqsign feature "reqwest_request"
    │           │   └── opendal feature "services-s3" (*)
    │           └── reqsign feature "services-aws"
    │               └── opendal feature "services-s3" (*)
    │       ├── reqwest feature "__rustls"
    │       │   └── reqwest feature "rustls-tls-webpki-roots-no-provider"
    │       │       └── reqwest feature "rustls-tls-webpki-roots"
    │       │           └── reqwest feature "rustls-tls"
    │       │               └── opendal feature "default" (*)
    │       ├── reqwest feature "__rustls-ring"
    │       │   └── reqwest feature "rustls-tls-webpki-roots" (*)
    │       ├── reqwest feature "__tls"
    │       │   └── reqwest feature "__rustls" (*)
    │       ├── reqwest feature "json"
    │       │   └── iceberg v0.4.0 (/Users/xxchan/Projects/iceberg-rust/crates/iceberg) (*)
    │       ├── reqwest feature "rustls-tls" (*)
    │       ├── reqwest feature "rustls-tls-webpki-roots" (*)
    │       ├── reqwest feature "rustls-tls-webpki-roots-no-provider" (*)
    │       └── reqwest feature "stream"
    │           └── opendal v0.51.2 (*)
    │   └── hyper-util feature "client-legacy"
    │       ├── hyper-rustls v0.27.5
    │       │   ├── hyper-rustls feature "http1"
    │       │   │   └── reqwest v0.12.12 (*)
    │       │   ├── hyper-rustls feature "ring"
    │       │   │   └── reqwest feature "__rustls-ring" (*)
    │       │   ├── hyper-rustls feature "tls12"
    │       │   │   └── reqwest v0.12.12 (*)
    │       │   ├── hyper-rustls feature "webpki-roots"
    │       │   │   └── hyper-rustls feature "webpki-tokio"
    │       │   │       └── reqwest feature "rustls-tls-webpki-roots-no-provider" (*)
    │       │   └── hyper-rustls feature "webpki-tokio" (*)
    │       └── reqwest v0.12.12 (*)
    ├── hyper-util feature "client-legacy" (*)
    ├── hyper-util feature "default"
    │   └── reqwest v0.12.12 (*)
    ├── hyper-util feature "http1"
    │   └── reqwest v0.12.12 (*)
    │   └── hyper-rustls feature "http1" (*)
    └── hyper-util feature "tokio"
        ├── hyper-rustls v0.27.5 (*)
        └── reqwest v0.12.12 (*)
        ├── hyper-util feature "client-legacy" (*)
        └── hyper-util feature "tokio" (*)
├── tokio feature "bytes"
│   └── tokio feature "io-util"
│       ├── opendal v0.51.2 (*)
│       └── parquet v54.2.0
│           ├── parquet feature "arrow"
│           │   └── parquet feature "default"
│           │       └── iceberg v0.4.0 (/Users/xxchan/Projects/iceberg-rust/crates/iceberg) (*)
│           ├── parquet feature "arrow-array"
│           │   └── parquet feature "arrow" (*)
│           ├── parquet feature "arrow-buffer"
│           │   └── parquet feature "arrow" (*)
│           ├── parquet feature "arrow-cast"
│           │   └── parquet feature "arrow" (*)
│           ├── parquet feature "arrow-data"
│           │   └── parquet feature "arrow" (*)
│           ├── parquet feature "arrow-ipc"
│           │   └── parquet feature "arrow" (*)
│           ├── parquet feature "arrow-schema"
│           │   └── parquet feature "arrow" (*)
│           ├── parquet feature "arrow-select"
│           │   └── parquet feature "arrow" (*)
│           ├── parquet feature "async"
│           │   └── iceberg v0.4.0 (/Users/xxchan/Projects/iceberg-rust/crates/iceberg) (*)
│           ├── parquet feature "base64"
│           │   ├── parquet feature "arrow" (*)
│           │   └── parquet feature "default" (*)
│           ├── parquet feature "brotli"
│           │   └── parquet feature "default" (*)
│           ├── parquet feature "default" (*)
│           ├── parquet feature "flate2"
│           │   └── parquet feature "default" (*)
│           ├── parquet feature "futures"
│           │   └── parquet feature "async" (*)
│           ├── parquet feature "lz4"
│           │   └── parquet feature "default" (*)
│           ├── parquet feature "lz4_flex"
│           │   └── parquet feature "lz4" (*)
│           ├── parquet feature "simdutf8"
│           │   └── parquet feature "default" (*)
│           ├── parquet feature "snap"
│           │   └── parquet feature "default" (*)
│           ├── parquet feature "tokio"
│           │   └── parquet feature "async" (*)
│           ├── parquet feature "zstd"
│           │   └── parquet feature "default" (*)
│           └── parquet feature "zstd-sys"
│               └── parquet feature "zstd" (*)
│       └── reqwest feature "__tls" (*)
├── tokio feature "default"
│   ├── backon v1.3.0
│   │   ├── backon feature "default"
│   │   │   └── opendal v0.51.2 (*)
│   │   ├── backon feature "gloo-timers-sleep"
│   │   │   └── backon feature "default" (*)
│   │   ├── backon feature "std-blocking-sleep"
│   │   │   └── backon feature "default" (*)
│   │   └── backon feature "tokio-sleep"
│   │       └── opendal v0.51.2 (*)
│   │       └── backon feature "default" (*)
│   ├── hyper v1.6.0
│   │   └── hyper-rustls v0.27.5 (*)
│   │   ├── hyper feature "client"
│   │   │   └── reqwest v0.12.12 (*)
│   │   │   └── hyper-util feature "client" (*)
│   │   ├── hyper feature "default"
│   │   │   ├── hyper-util v0.1.10 (*)
│   │   │   └── reqwest v0.12.12 (*)
│   │   └── hyper feature "http1"
│   │       └── reqwest v0.12.12 (*)
│   │       └── hyper-util feature "http1" (*)
│   ├── hyper-rustls v0.27.5 (*)
│   ├── opendal v0.51.2 (*)
│   ├── tokio-rustls v0.26.1
│   │   └── hyper-rustls v0.27.5 (*)
│   │   ├── tokio-rustls feature "ring"
│   │   │   └── reqwest feature "__rustls-ring" (*)
│   │   └── tokio-rustls feature "tls12"
│   │       └── reqwest v0.12.12 (*)
│   │       └── hyper-rustls feature "tls12" (*)
│   ├── tokio-util v0.7.13
│   │   ├── tokio-util feature "codec"
│   │   │   └── reqwest v0.12.12 (*)
│   │   └── tokio-util feature "io"
│   │       └── reqwest v0.12.12 (*)
│   └── tower v0.5.2
│       ├── tower feature "__common"
│       │   └── tower feature "util"
│       │       └── reqwest v0.12.12 (*)
│       ├── tower feature "futures-core"
│       │   └── tower feature "__common" (*)
│       ├── tower feature "futures-util"
│       │   └── tower feature "util" (*)
│       ├── tower feature "pin-project-lite"
│       │   ├── tower feature "__common" (*)
│       │   ├── tower feature "timeout"
│       │   │   └── reqwest v0.12.12 (*)
│       │   └── tower feature "util" (*)
│       ├── tower feature "sync_wrapper"
│       │   └── tower feature "util" (*)
│       ├── tower feature "timeout" (*)
│       ├── tower feature "tokio"
│       │   └── tower feature "timeout" (*)
│       └── tower feature "util" (*)
├── tokio feature "fs"
│   ├── opendal feature "services-fs" (*)
│   └── reqwest feature "stream" (*)
├── tokio feature "io-util" (*)
├── tokio feature "libc"
│   └── tokio feature "net"
│       └── reqwest v0.12.12 (*)
│       └── hyper-util feature "tokio" (*)
├── tokio feature "macros"
│   └── parquet v54.2.0 (*)
├── tokio feature "mio"
│   └── tokio feature "net" (*)
├── tokio feature "net" (*)
├── tokio feature "rt"
│   └── parquet v54.2.0 (*)
│   ├── opendal feature "executors-tokio" (*)
│   ├── tokio feature "rt-multi-thread"
│   │   └── opendal feature "internal-tokio-rt" (*)
│   └── hyper-util feature "tokio" (*)
├── tokio feature "rt-multi-thread" (*)
├── tokio feature "socket2"
│   └── tokio feature "net" (*)
├── tokio feature "sync"
│   ├── hyper v1.6.0 (*)
│   ├── iceberg v0.4.0 (/Users/xxchan/Projects/iceberg-rust/crates/iceberg) (*)
│   ├── opendal v0.51.2 (*)
│   ├── tokio-util v0.7.13 (*)
│   └── tower v0.5.2 (*)
│   └── hyper-util feature "client-legacy" (*)
├── tokio feature "time"
│   └── reqwest v0.12.12 (*)
│   ├── backon feature "tokio-sleep" (*)
│   ├── hyper-util feature "tokio" (*)
│   └── tower feature "timeout" (*)
└── tokio feature "tokio-macros"
    └── tokio feature "macros" (*)

[xxchan]

To summarize, currently we already enable tokio runtime features unconditionally, even when feature tokio is not enabled, since other dependencies enabled it.

Therefore, I think the best way to proceed is:

• In this PR, we just need to remove the "optional" for tokio to unblock feat(puffin): Make Puffin APIs public #1165.
• In a follow-up PR, we can gradually turn off unnecessary features of dependencies.

[xxchan]

To summarize, currently we already enable tokio runtime features unconditionally, even when feature tokio is not enabled, since other dependencies enabled it.

Actually tokio is a hard requirement for opendal now? 🤔 Since executors-tokio is the only available executor now. @Xuanwo

[liurenjie1024]

To summarize, currently we already enable tokio runtime features unconditionally, even when feature tokio is not enabled, since other dependencies enabled it.

Therefore, I think the best way to proceed is:

• In this PR, we just need to remove the "optional" for tokio to unblock feat(puffin): Make Puffin APIs public #1165.
• In a follow-up PR, we can gradually turn off unnecessary features of dependencies.

I agree that we should eventually remove unnecessary tokio runtime dependency, but I don't think we should revert the change of tokio/rt-multi-thread, as this is the goal we want to achieve. Thinking about the final status, it should be what looks like in this pr.

[Xuanwo]

Thank you @liurenjie1024, let's unblock other PRs. We can process #1036 later.