Skip to content

Fix shared file 4k read#106

Open
JulianKunkel wants to merge 4 commits intomainfrom
fix-shared-file-4k-read
Open

Fix shared file 4k read#106
JulianKunkel wants to merge 4 commits intomainfrom
fix-shared-file-4k-read

Conversation

@JulianKunkel
Copy link
Copy Markdown
Contributor

Supporting singe shared file and shifting random read away from previously written node.

@JulianKunkel JulianKunkel requested a review from a team as a code owner April 1, 2026 06:37
adilger
adilger previously approved these changes Apr 1, 2026
Copy link
Copy Markdown
Contributor

@adilger adilger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should wait on landing until testing has confirmed this fixes the issue.

@@ -44,6 +44,12 @@ static double run(void){
u_argv_push(argv, "-s=10000000");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As previously discussed, it would be good to add a separate patch that sets -s=(file_size / (10 * 4096)) (i.e. 10% of file size) so that the random reads don't have too many cache hits. The sqrt(file_size / 4096) is the chance of one collision with random reads, and we could work out the math so that the random read phase exits when the chance of repeated collisions is too high. 10% of the file size is just a random guess.

Comment thread prepare.sh
echo It will output OK at the end if builds succeed
echo
IOR_HASH=8ab8f69b32b919
IOR_HASH=4a34dfc
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A slightly longer hash would be preferred (10-12 chars). At only 7 chars this is likely to collide after only 16k commits in IOR. While not an issue today (about 1k commits in IOR), it is better to think about this for the future.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, I do use the hash proposed by GitHub to id the version.

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.

3 participants