[
https://issues.apache.org/jira/browse/COUCHDB-1398?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13207768#comment-13207768
]
Filipe Manana commented on COUCHDB-1398:
----------------------------------------
Hi Benoît,
A few comments after quickly looking at the full diff (sorry had no time to
test).
1) There seems to be a few white space only unintentional changes that don't
eliminate trailing white spaces or
fix the indentation to comply to the overall indentation style. E.g.:
https://github.com/benoitc/couchdb/compare/master...couch_view_changes#L4L357
2) In the make_update_fun:
https://github.com/benoitc/couchdb/compare/master...couch_view_changes#L4R114
I see a few issues there.
First, if the design document is deleted (the view group is shutdown), and the
_changes feed request is of "continuous" type, the client will hang forever as
the code doesn't seem to handle this
and will be forever waiting for more 'view_updated' events which will never
come (unless a new ddoc with same _id
is created after).
Second, when it receives a db_updated event, it will spawn a process to trigger
the view update. If this process dies,
the parent doesn't know it and thinks all is fine. It can die here because for
example
couch_httpd_db:couch_doc_open threw {not_found, deleted} (ddoc deleted, related
to case 1 mentioned above).
It should stop when the ddoc is deleted or a ddoc update removes its "views"
field (which also shutdowns the view group
as soon as no more clients are using it). Possibly when a view group is
shutdown, you'll want to emit an event so that
you can react to it. Inside that process you're also calling couch_db:reopen -
you should use couch_db:open[_int] here
because that process never opened the database before.
Third, and this might not be an issue (hard to tell) is when for example the
ddoc signature changes (like map function was
updated), you keep going as nothing happened. Clients might want to stop
receiving changes if the view definitions of
a ddoc change - this might be very subjective or application dependent. I don't
have a strong opinion here.
As of COUCHDB-1309, when a ddoc is updated such that the view group signature
changes, it's old view group will
shutdown when no more clients are using it. Perhaps when it's shutdown,
emitting an event with the DDocId and Signature
will help dealing with the cases listed above.
First case where a ddoc is deleted, should probably have a test to verify the
clients isn't kept blocked forever
after the ddoc is deleted (for continuous feed types).
3) You're reusing couch_db_update_notifier for emitting the view_updated
events. Probably the couch_mrview application
should have it's own separate event manager. I don't see this as a big issue or
a blocker, can be addressed later.
4) For the replication, some tests would be very welcome and good to have.
Good work and thanks.
> improve view filtering in changes
> ---------------------------------
>
> Key: COUCHDB-1398
> URL: https://issues.apache.org/jira/browse/COUCHDB-1398
> Project: CouchDB
> Issue Type: Improvement
> Components: View Server Support
> Affects Versions: 2.0, 1.3
> Reporter: Benoit Chesneau
> Labels: changes, view
> Attachments: 0001-white-spaces.patch,
> 0002-initial-step-move-the-code-from-couch_httpd_db-to-co.patch,
> 0003-fix-indent.patch,
> 0004-This-wrapper-is-useless-somehow-split-the-code-in-a-.patch,
> 0005-add-view_updated-event.patch, 0006-immprove-view-filter.patch,
> 0007-useless-info.patch, 0008-whitespaces.patch,
> 0009-handle-native-filters-in-views.patch
>
>
> Improve the native view filter `_view` support by really using view index.
> This patches add following features
> - small refactoring: create the couch_httpd_changes modules, to put all the
> changes http support in its own module instead having it in couch_httpd_db.
> - add the `view_updated` event when a view index is updated : {view_updated,
> {DbName, IndexName}}
> - start the feed using results in the view index instead of all the db index
> - only react on view index changes.
> For now next changes are still get using the couch_db:changes_since function
> and passing the map function to the results. It could be improved if we had a
> by_seq btree in the view index too. Other way may be to skip a number of the
> documents already processed. Not sure it would be faster. Thoughts ?
> The branch couch_view_changes in my repo contains preliminary support:
> https://github.com/benoitc/couchdb/tree/couch_view_changes
> Diff:
> https://github.com/benoitc/couchdb/compare/master...couch_view_changes
> To use it, use the native filter named _view which take the parameter
> view=DesignName/Viewname
> eg:
>
> http://server/db/_changes?feed=continuous&heartbeat=true&filter=_view&view=DesignName/SomeView
> It has also an interresting side effect: on each db updates the view index
> refresh is triggered so view updates are triggered. Maybe we could introduce
> an optionnal parameter to not trigger them though?
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
