Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[Reducing DB interactions at mongoBackend] Avoid update csub when notifications are sent #1476

Closed
fgalan opened this issue Nov 5, 2015 · 4 comments
Labels
Milestone

Comments

@fgalan
Copy link
Member

fgalan commented Nov 5, 2015

Currently, each time a notification is sent, the csub is updated to increase count and set the new lastNotification. This is inefficent.

A better approach should be implemented. A couple of ideas:

  • Not using at all count or lastNotification in DB (although it could be a problem considering the current NGSIv2 JSON for subscriptions, which takes into account both fields)
  • Accumulate all the changes (maybe using the cache structures?) and consolidate in DB in a regular basic (maybe at cache refresh frecuency?). The drawback is the risk of data lost and eventual consistency in the case of Active-Active configurations.
@fgalan
Copy link
Member Author

fgalan commented Nov 20, 2015

Considering option 2 (accumulate and cosolidate at refresh time) a solution is to use one update() for each field:

To update count, being <n> the amount to sum to DB:

update({ _id: ObjectId(<subId>)}, {count: {$inc: <n>}})

To update lastNotified, being the last notification time:

update({ _id: ObjectId(<subId>), lastNotification: {$lt: <lnTime>}}, {lastNotification: <lnTime>}})

(Not sure if the above is 100% correct syntax)

Actually, the above is expressed in mongo query language. In our code, the collectionUpdate() function has to be used. The first argument of the update() above should be mapped to a BSONObj in the q argument, while the second argument should be mapped to a BSONObj in the doc argument. upsert should be set to false (it is assumed that the document in the csub collection already exists).

bool collectionUpdate
(
  const std::string&  col,
  const BSONObj&      q,
  const BSONObj&      doc,
  bool                upsert,
  std::string*        err
)

Of course 2 update() is worse than 1 update() :). However, I cannot figure out how to do this with just one operation. Maybe an important optimization would be to use a bulk update to update all the cache documents: https://docs.mongodb.org/manual/reference/method/Bulk.find.updateOne/#Bulk.find.updateOne (not essential for #1476 completion, as we can assume that refresh wouldn't be too frecuent in production cases, i.e. in the order of tens of senconds; an issue should be opened about it).

As additional comment -subCacheIval 0 is no longer a good idea, as count/lastNotification should be sync at DB from time to time (to prevent that a CB crash lost too much information). In addition, orionExit() function should execute the refresh part to sync count/lastNotification fields.

(Final thought: maybe "refresh" is not a good name for the operation. At least I tend to think in "refresh" as an exclusive DB -> cache operation. Maybe "sync" is better. Not sure anyway).

@kzangeli
Copy link
Member

Yeah, I think mongoSubCacheSync might be a better name when this is included.

Now, the current mongoSubCacheRefresh* function throws away the cache and replaces it with what was found in the db. When lastNotificationTime and count is no longer maintained in the DB, this will have to be changed. mongoSubCacheRefresh must get smarter.

  1. Add count to CachedSubscriptions, set it to 0 after each mongoSubCacheSync (to count the increment only)
  2. Before mongoSubCacheRefresh, save lastNotificationTime and count for each notification in the cache (as a vector of struct { subId, lastNotificationTime, count } ), after that
  3. refresh from DB, and then,
  4. compare lastNotificationTime from the "new" cache to the "old" value - the "old" value might be more recent than what was found in the DB ...
  5. These lastNotificationTime must now be flushed to DB and the accumulated counts also.

@fgalan
Copy link
Member Author

fgalan commented Nov 25, 2015

Implemented in PR #1536. However, some modifications has bene deferred related with -noCache. From comments in that PR:

@fgalan
Copy link
Member Author

fgalan commented Nov 26, 2015

Finished in PR #1544

@fgalan fgalan closed this as completed Nov 26, 2015
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants