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

ignore (workaround for lack of itest runability) #11987

Closed
wants to merge 42 commits into from

Conversation

ribasushi
Copy link
Collaborator

@ribasushi ribasushi commented May 11, 2024

DO NOT MERGE
DO NOT LOOK
KEEP WALKING

☨ this PR is never meant to leave draft status: it is merely temporary CI for https://github.com/ribasushi/ltsh#readme, as running itests locally is currently quite challenging

@ribasushi ribasushi force-pushed the master branch 6 times, most recently from 795c1ce to 1e10e2b Compare May 16, 2024 09:18
@rvagg
Copy link
Member

rvagg commented May 30, 2024

OK, but just be aware that the cost of running these tests is coming off a certain budget; it's good to be supporting the ecosystem, but bills will add up if this is going to be a regular thing.

@ribasushi
Copy link
Collaborator Author

I am very aware. A friendly suggestion: instead of trying to make the ecosystem feel bad about your "limited" budget: fix your tests to be runnable by mere mortals.

@ribasushi ribasushi changed the base branch from release/v1.26.4 to master June 15, 2024 18:17
@ribasushi ribasushi changed the base branch from master to release/v1.27.1 June 15, 2024 19:21
@ribasushi ribasushi force-pushed the master branch 7 times, most recently from c5ffb74 to 85ae603 Compare June 20, 2024 23:07
ribasushi and others added 17 commits July 17, 2024 18:22
This is a large diff, yet should have exactly zero functional changes

Ideally as a result of this some parts of the depchain will become lighter,
with downstream reaping the same benefits as the team that initiated this split.

P.S. work was done while forming better intuition of current dependency graph
storage/pipeline.NewPreCommitBatcher and storage/pipeline.New now have an additional
error return to deal with errors arising from fetching the sealing config.
error return to deal with errors arising from fetching the sealing config.
There are no perf gains to be had here anyway
By a wide margin the top issue with large blockstores is the size of their index.

Transparently switch the badger lookup key to the leading 128 bits of the hash,
regardless of type/codec. In the proccess forbid AllKeysChan and rewrite ForEachKey.

Also unconditionally enable HashOnRead (only safe way to do partial keys)

This is safe (and has been tested) to run on an existing lotus install, with splitstore
enabled and compacting properly. When converting an existing setup simply set the
envvar LOTUS_CHAIN_BADGERSTORE_QUERY_LEGACY_KEYS to true. When sufficient amount of
FullGC cycles have passed and all the keys in your blockstore are "new" - the envvar
is no longer needed.

Reasoning:

The filecoin blockstore in April 2024 sits at about 26 billion blocks. The
current scheme of /blocks/base32-enc-of-entire-mh puts a key at 66 bytes, for
a total raw index of about 1.6TiB.

Reduction of every key to mere 16 bytes reduces this to just 390GiB, potentially
even fitting in memory.
Prefix every bin-key value with a single uint64 varint containing:
  (3bit store-type)            // 0 for as-is, 1 for basic zstd compression, remainder reserved
    +
  (one reserved bit)     << 3  // always 0 for now
    +
  (3bit compressability) << 4  // 0 if store-type is 0, otherwise `c := (origLen-compLen) * 8 / origLen`
                               // (using integer math, `origLen > compLen > 0` holds for any non-0 store-type)
    +
  (IPLD block data size) << 7  // 0 if store-type is 0

Include a rudimentary, dictionary-less zstd compressor as the first
non-verbatim storage type 1
ribasushi and others added 4 commits July 17, 2024 18:52
There is no value for a node to participate in relaying any of this
(nor is there much positive value for the network as a whole...)
After this commit a node's entire libp2p Hello looks like:

/chain/ipfs/bitswap
/chain/ipfs/bitswap/1.0.0
/chain/ipfs/bitswap/1.1.0
/chain/ipfs/bitswap/1.2.0
/fil/chain/xchg/0.0.1
/fil/hello/1.0.0
/floodsub/1.0.0
/ipfs/id/1.0.0
/ipfs/id/push/1.0.0
/ipfs/ping/1.0.0
/meshsub/1.0.0
/meshsub/1.1.0
@jennijuju
Copy link
Member

I am closing this PR as it doesn't seem like it is contributing to this code base.

@jennijuju jennijuju closed this Jul 18, 2024
@ribasushi
Copy link
Collaborator Author

@jennijuju so... this is a weird action. I was told that the real fix for the title of this PR (inability to successfully run go test -count=1 ./...) is not something FilOz is interested in prioritizing any time soon.

Therefore this leaves me 2 choices

  1. keep this known-by-everyone-to-ignore PR in a perpetual draft to run tests
  2. keep opening new random draft PRs every time I need to run this, cluttering your already busy comms channels

That you are forcing me to do 2. is... strange 🤷

@jennijuju
Copy link
Member

@ribasushi or there is another option that maybe you can help contribute to the code base, that you are use as an upstream, to improve how test can be run

@jennijuju
Copy link
Member

Or have you considered to not open draft PRs knowing that they are “fake PRs” with no intention to get them landed in the code base, but actually open those PR with a goal to land them(contribute to OSS public good)?

@ribasushi
Copy link
Collaborator Author

actually open those PR with a goal to land them(contribute to OSS public good)?

@jennijuju I am sorry, but you really have no standing to lecture me on how "OSS public good" works, or what it even is.

The reason I spend a considerable amount of time to maintain a side-repo in the 1st place is because your team is notoriously obstructionist when it comes to accepting non-trivial changes from experienced operators. Hence I open PRs for what reasonably has a chance to land, and deal on my own with stuff I gave up arguing about. Some of the changes I carry in my tree are literally from the year 2020. All the code is there, ready to go, when someone who recognizes OSS is a two-way street comes along to pick them up.

ribasushi/ltsh@backports...maybe_for_upstreaming

image

In the meantime - message received: if one needs to execute all the tests (regardless of reason) then "fake" PRs on as-neede basis it is.

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.

7 participants