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

Insert in tx #51

Closed
wants to merge 52 commits into from
Closed

Insert in tx #51

wants to merge 52 commits into from

Conversation

lyoshenka
Copy link
Member

@lyoshenka lyoshenka commented May 20, 2021

my version of @shyba's #50. see which you like better (or maybe you'll have an even better idea?)

wrap blob insertion in tx. fixes lbryio/lbry-sdk#3296

The problem is that inserting an sd blob with ~5k
blobs takes longer than 30 seconds. So the client
times out and retries the request. At that point,
reflector is not done inserting so it replies with
a smaller number of blobs than it should. The client
uploads that many blobs and marks the stream as
reflected. The remaining blobs never get uploaded.

Victor says doing the insert inside a transaction should be
faster than doing 10k (2 per blob) inserts
independently.

He's also doing a client-side fix for this.

nikooo777 and others added 30 commits November 27, 2020 16:19
don't wait for a blob to be written to disk before sending it downstream
don't wait for the disk store to be walked before starting everything up
update quic
fix tests
swap size to bytes
increase idle timeout to avoid errors downstream
add option to delete blobs from DB if storage doesn't have it (for future local tracking)
fix store init
try fixing unreasonable db bottleneck
reduce touch time to every 6 hours
add cache for blobs not found
@shyba
Copy link
Member

shyba commented May 20, 2021

client side fix at lbryio/lbry-sdk#3308

if we can make that faster it should also help not having incomplete data on shutdown.
I had a similar issue on the SDK some years ago and using a transaction was absurdly faster, but I don't know how much that replicates to MySQL

db/ifaces.go Outdated Show resolved Hide resolved
db/ifaces.go Outdated Show resolved Hide resolved
db/db.go Outdated Show resolved Hide resolved
@anbsky
Copy link
Contributor

anbsky commented May 21, 2021

I'm in favor of this implementation. Aside from a few nitpicks, I don't have any qualms with it.

@lyoshenka lyoshenka force-pushed the insert_in_tx branch 2 times, most recently from 0470b73 to b404458 Compare May 21, 2021 18:47
nikooo777 and others added 2 commits May 21, 2021 21:06
The problem is that inserting an sd blob with ~5k
blobs takes longer than 30 seconds. So the client
times out and retries the request. At that point,
reflector is not done inserting so it replies with
a smaller number of blobs than it should. The client
uploads that many blobs and marks the stream as
reflected. The remaining blobs never get uploaded.

Doing the insert inside a transaction should be
faster than doing 10k (2 per blob) inserts
independently.
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.

Not all blobs accounted for on reflection for larger files
6 participants