Skip to content

batch appends - #5

Open
mvayngrib wants to merge 1 commit into
mafintosh:masterfrom
mvayngrib:batch
Open

batch appends#5
mvayngrib wants to merge 1 commit into
mafintosh:masterfrom
mvayngrib:batch

Conversation

@mvayngrib

Copy link
Copy Markdown
Collaborator

based on #4

@mafintosh

Copy link
Copy Markdown
Owner

@mvayngrib thanks for all the PRs. I'll go through them all over the weekend :)

@mvayngrib

Copy link
Copy Markdown
Collaborator Author

@mafintosh great, thanks

Comment thread index.js
key: item.key,
value: item.value
}
}), function(err) {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

if the feed fails do we reset the change count?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

good point

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

but if one batch fails and meanwhile the next succeeds? Resetting might make things even weirder. I guess this is where mutefixy kept things simple

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

yea this gets a bit tricky fast. maybe we should add some tests (somehow) for this so we can make we hit all the edge-cases

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

i think the simplest solution is:

  1. set a flag that we are running a batch.
  2. if flag is set append to a known tmp batch
  3. after 1 completes unset flag and if tmp batch is not empty goto 1 and run that batch

basically a mutex batch (should be super fast)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

right, the tmp batch we already have, it's feed.batch. We just need a lock around the commit so there is never the possibility of commit 2 succeeding while commit1 fails.

the question is what to do when the current batch fails, and there's also a tmp batch in the balance

@mafintosh

Copy link
Copy Markdown
Owner

@mvayngrib btw, do you want to help maintain this module? i can add you as collaborator if you want :)

@mvayngrib

Copy link
Copy Markdown
Collaborator Author

@mafintosh yes, add me please :)

@mafintosh

Copy link
Copy Markdown
Owner

@mvayngrib added. whats your npm username?

@mvayngrib

Copy link
Copy Markdown
Collaborator Author

@mafintosh npm username: tenaciousmv (also, see line note above re: batch locks)

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.

2 participants