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.

Sign up for GitHub

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

Jump to bottom

Buffered livedata fix #167

Merged
merged 3 commits into from
Apr 21, 2016
Merged

Buffered livedata fix #167

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d9d9a4b
Tests need to be async since the livedata updates might not happen in…
abernix Apr 21, 2016
2b3004d
If the connection supports flushing the livedata buffer, do it.
abernix Apr 21, 2016
ff7ff6d
[Optional] The new livedata seems to handle waitForQuiescence fine
abernix Apr 21, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 17 additions & 5 deletions lib/client/fast_render.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,17 @@ if(FastRender._disable) {
// This is the cure
FastRender.injectDdpMessage = function(conn, message) {
FastRender["debugger"].log('injecting ddp message:', message);
var originalWait = conn._waitingForQuiescence;
conn._waitingForQuiescence = function() {return false};
conn._livedata_data(message);
conn._waitingForQuiescence = originalWait;
if (conn._bufferedWrites) {
Copy link

@tmeasday tmeasday Aug 10, 2016 •
edited

Choose a reason for hiding this comment

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

I think this change should not have been included because if the connection is _waitingForQuiescence (for instance if a login method is run on startup), the writes get placed into a _messagesBufferedUntilQuiescence queue.

Those messages will not get into the _bufferedWrites queue until quiescence happens (the method returns from the server). So the later _flushBufferedWrites() will not pick them up.

See (meteor/meteor#7618)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure why I had things working at some point, but indeed, that makes sense. Looks like @dnish already got the fix applied for this. Good catch!

// New with meteor/meteor#5680
// If the livedata connection supports buffered writes,
// we don't need check if we're in delay before injecting.
conn._livedata_data(message);
} else {
var originalWait = conn._waitingForQuiescence;
conn._waitingForQuiescence = function() {return false};
conn._livedata_data(message);
conn._waitingForQuiescence = originalWait;
}
};

FastRender.init = function(payload) {
Expand Down Expand Up @@ -56,6 +63,8 @@ FastRender.init = function(payload) {
});
}

var connection = Meteor.connection;

_.each(allData, function(collData, collName) {
_.each(collData, function(item, id) {
var id = IDTools.idStringify(item._id);
Expand All @@ -69,10 +78,13 @@ FastRender.init = function(payload) {
frGen: true
};

FastRender.injectDdpMessage(Meteor.connection, ddpMessage);
FastRender.injectDdpMessage(connection, ddpMessage);
});
});

// If the connection supports buffered DDP writes, then flush now.
if (connection._flushBufferedWrites) connection._flushBufferedWrites();

// let Meteor know, user login process has been completed
if(typeof Accounts != 'undefined') {
Accounts._setLoggingIn(false);
Expand Down
31 changes: 20 additions & 11 deletions tests/client/ddp_update.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
Tinytest.add('DDPUpdate - convert added to changed', function(test) {
Tinytest.addAsync('DDPUpdate - convert added to changed', function(test, done) {
var collName = Random.id();
var coll = new Mongo.Collection(collName);

Expand All @@ -9,19 +9,25 @@ Tinytest.add('DDPUpdate - convert added to changed', function(test) {
fields: {name: "arunoda"}
});

test.equal(coll.findOne('one'), {_id: 'one', name: 'arunoda'});
Meteor.setTimeout(function () {
test.equal(coll.findOne('one'), {_id: 'one', name: 'arunoda'});

Meteor.connection._livedata_data({
msg: 'added',
collection: collName,
id: 'one',
fields: {name: "kuma", age: 20}
});
Meteor.connection._livedata_data({
msg: 'added',
collection: collName,
id: 'one',
fields: {name: "kuma", age: 20}
});

Meteor.setTimeout(function () {
test.equal(coll.findOne('one'), {_id: 'one', name: 'kuma', age: 20});
done();
}, bufferedWritesInterval);

test.equal(coll.findOne('one'), {_id: 'one', name: 'kuma', age: 20});
}, bufferedWritesInterval);
});

Tinytest.add('DDPUpdate - create collection later on', function(test) {
Tinytest.addAsync('DDPUpdate - create collection later on', function(test, done) {
var collName = Random.id();

Meteor.connection._livedata_data({
Expand All @@ -39,7 +45,10 @@ Tinytest.add('DDPUpdate - create collection later on', function(test) {
});

var coll = new Mongo.Collection(collName);
test.equal(coll.find().fetch().length, 2);
Meteor.setTimeout(function () {
test.equal(coll.find().fetch().length, 2);
done();
}, bufferedWritesInterval);
});

Tinytest.add('DDPUpdate - delete subscriptions', function(test) {
Expand Down
47 changes: 28 additions & 19 deletions tests/client/fast_render.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ Tinytest.addAsync('FastRender - init - ObjectId support', function(test, done) {
});
});

Tinytest.add('FastRender - init - merge docs', function(test) {
Tinytest.addAsync('FastRender - init - merge docs', function(test, done) {
var collName = Random.id();
var payload = {
subscriptions: {posts: true},
Expand All @@ -69,16 +69,19 @@ Tinytest.add('FastRender - init - merge docs', function(test) {
FastRender.init(payload);

var coll = new Mongo.Collection(collName);
test.equal(coll.findOne('one'), {
_id: "one",
name: "arunoda",
age: 30,
city: "colombo",
plan: "pro"
});
Meteor.setTimeout(function () {
test.equal(coll.findOne('one'), {
_id: "one",
name: "arunoda",
age: 30,
city: "colombo",
plan: "pro"
});
done();
}, bufferedWritesInterval);
});

Tinytest.add('FastRender - init - merge docs deep', function(test) {
Tinytest.addAsync('FastRender - init - merge docs deep', function(test, done) {
var collName = Random.id();
var payload = {
subscriptions: {posts: true},
Expand All @@ -95,18 +98,21 @@ Tinytest.add('FastRender - init - merge docs deep', function(test) {
FastRender.init(payload);

var coll = new Mongo.Collection(collName);
test.equal(coll.findOne('one'), {
_id: "one",
name: "arunoda",
profile: {
Meteor.setTimeout(function () {
test.equal(coll.findOne('one'), {
_id: "one",
name: "arunoda",
email: "arunoda@arunoda.com"
}
});
profile: {
name: "arunoda",
email: "arunoda@arunoda.com"
}
});
done();
}, bufferedWritesInterval);
});


Tinytest.add('FastRender - init - ejon data', function(test) {
Tinytest.addAsync('FastRender - init - ejon data', function(test, done) {
var collName = Random.id();
var payload = {
subscriptions: {posts: true},
Expand All @@ -123,8 +129,11 @@ Tinytest.add('FastRender - init - ejon data', function(test) {
FastRender.init(payload);

var coll = new Mongo.Collection(collName);
var doc = coll.findOne("one");
test.equal(doc.date.getTime(), date.getTime());
Meteor.setTimeout(function () {
var doc = coll.findOne("one");
test.equal(doc.date.getTime(), date.getTime());
done();
}, bufferedWritesInterval);
});

WithNewInjectDdpMessage = function(newCallback, runCode) {
Expand Down
2 changes: 2 additions & 0 deletions tests/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
bufferedWritesInterval = 5;

Tinytest.add('AddedToChanged - new fields', function(test) {
var localCopy = {aa: 10};
var added = {fields: {aa: 20, bb: 20}, msg: 'added'};
Expand Down