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

Replication rejects attachments generated by PouchDB with revpos:0 #1200

Closed
shadzik opened this issue Apr 5, 2016 · 8 comments
Closed

Replication rejects attachments generated by PouchDB with revpos:0 #1200

shadzik opened this issue Apr 5, 2016 · 8 comments
Assignees
Milestone

Comments

@shadzik
Copy link

shadzik commented Apr 5, 2016

16:31:20.223‖ WARNING: CBLRestPuller[https://server:443/user-62356431623932312d323233652d343935612d623834622d663439666462373234356336]: Revision {section_image-7d5198c2-5ddf-5405-9835-e980b634535c #2-8145a6217ced16862b43577c204d2ac8} has invalid attachment metadata: {
    "image.png" =     {
        "content_type" = "image/png";
        digest = "md5-Hke94Wxx368Us7xYprTmIw==";
        follows = 1;
        length = 101033;
        revpos = 0;
    };
}

The issue seems to be revpos not having a proper value, value is 0 and should be 2 in this case.
But anyway i would expect this doc to be synced.


  • Version: 1.2.0
  • Client OS: iOS 9.3
  • Server: Apache CouchDB 1.5.0
@jo
Copy link

jo commented Apr 5, 2016

I can confirm that the abovementioned doc has been successfully replicated using CouchDB 1.6.1 replicator.

@jo
Copy link

jo commented Apr 5, 2016

Some additional info

The warning is triggered here: https://github.com/couchbase/couchbase-lite-ios/blob/master/Source/CBLRestPuller.m#L711-L714

We don't know how the doc with the bogus revpos has been created yet.

@jo
Copy link

jo commented Apr 6, 2016

Why is revpos == 0 considered invalid?

jo pushed a commit to pouchdb/pouchdb that referenced this issue Apr 6, 2016
Add the `revpos` property to the attachment stub. The `revpos` is the doc's
`rev` prefix (a sequential number) from when the attachment was added or
updated. It is used during replication in the CouchDB replicator (and others
including CouchBase Lite) to avoid copying attachments that have not changed
since the last replication.

This solves a problem with documents with attachments created in PouchDB, which
are later replicated to an Apache CouchDB. If the doc is then pulled from an iOS
application using CouchBase Lite the doc is skipped, because CBL rejects the
attachments because of invalid attachment stub (see
couchbase/couchbase-lite-ios#1200)

Also this improves replication performance of documents created by PouchDB,
which were later synced using Apache CouchDB replicator.
jo pushed a commit to pouchdb/pouchdb that referenced this issue Apr 6, 2016
Add the `revpos` property to the attachment stub. The `revpos` is the doc's
`rev` prefix (a sequential number) from when the attachment was added or
updated. It is used during replication in the CouchDB replicator (and others
including CouchBase Lite) to avoid copying attachments that have not changed
since the last replication.

This solves a problem with documents with attachments created in PouchDB, which
are later replicated to an Apache CouchDB. If the doc is then pulled from an iOS
application using CouchBase Lite the doc is skipped, because CBL rejects the
attachments because of invalid attachment stub (see
couchbase/couchbase-lite-ios#1200)

Also this improves replication performance of documents created by PouchDB,
which were later synced using Apache CouchDB replicator.
@snej
Copy link
Contributor

snej commented Apr 6, 2016

revpos indicates the generation number (numeric prefix in the revID) at which the attachment was last altered. Generation numbers start at 1 when a new doc is created, so a revpos of 0 refers to an invalid generation number.

(I believe the sanity check will accept an attachment with no revpos, but an explicit value of 0 as in this case indicates something is wrong.)

@jo
Copy link

jo commented Apr 6, 2016

While I totally agree that revpos 0 does not make any sense at all this behaviour breaks replication with a CouchDB, which has docs with attachments generated by PouchDB.
What would be the impact if CBL would consider revpos 0 as valid like it does with missing revpos?

@snej
Copy link
Contributor

snej commented Apr 6, 2016

We can figure out a workaround — treating revpos:0 as equivalent to a missing revpos seems reasonable.

But I'm curious why this is only happening now, when Couchbase Lite has been interoperating with CouchDB since 2011. It sounds like it's PouchDB that generates the revisions with revpos:0, and CouchDB is just the intermediary? And it looks like the PouchDB commits referenced above fix the issue?

@snej snej changed the title Replication does not save attachments with invalid metadata Replication rejects attachments generated by PouchDB with revpos:0 Apr 6, 2016
@jo
Copy link

jo commented Apr 6, 2016

Hi @snej, thanks for the discussion!

Yes, the revpos: 0 entries are indeed caused by PouchDB, or at least this happens during standard CouchDB replication of documents created by PouchDB. PouchDB itself ignores revpos atm and I am indeed working on a pull request to bring revpos support to PouchDB.

In pouchdb/pouchdb#3930 there is also a discussion about this topic btw.

But even if PouchDB will support revpos in the future there will be still documents in place which are created using current or older PouchDB versions. I think its a good idea to not break replication of those docs. And CouchDB itself is also a bit more relaxed in this case and is able to replicate such docs.

jo pushed a commit to pouchdb/pouchdb that referenced this issue Apr 7, 2016
Add the `revpos` property to attachment stubs on save. `revpos` is the doc's
`rev` prefix (a sequential number) from when the attachment was added or
updated. It is used during replication in the CouchDB replicator (and others
including CouchBase Lite) to avoid fetching attachments that have not changed
since last replication.

This solves a problem with attachments created in PouchDB, which are then
replicated to an Apache CouchDB instance. They end up having a `revpos` property
set to `0`. When the doc is later pull replicated from an iOS application using
CouchBase Lite it is skipped and does not get replicated. This is because CBL
rejects attachments with invalid attachment stub (see
couchbase/couchbase-lite-ios#1200). `revpos:0` is
considered invalid in CBL.

Also this improves replication performance of documents with attachments created
by PouchDB, when they are later replicated using Apache CouchDBs own replicator.
The Apache CouchDB replicator itself handles `revpos:0` docs without a problem.
But the replication of those docs cannot be optimized; the attachments get
copied over again for every revision, even if they did not change.
jo pushed a commit to pouchdb/pouchdb that referenced this issue Apr 7, 2016
Add the `revpos` property to attachment stubs on save. `revpos` is the doc's
`rev` prefix (a sequential number) from when the attachment was added or
updated. It is used during replication in the CouchDB replicator (and others
including CouchBase Lite) to avoid fetching attachments that have not changed
since last replication.

This solves a problem with attachments created in PouchDB, which are then
replicated to an Apache CouchDB instance. They end up having a `revpos` property
set to `0`. When the doc is later pull replicated from an iOS application using
CouchBase Lite it is skipped and does not get replicated. This is because CBL
rejects attachments with invalid attachment stub (see
couchbase/couchbase-lite-ios#1200). `revpos:0` is
considered invalid in CBL.

Also this improves replication performance of documents with attachments created
by PouchDB, when they are later replicated using Apache CouchDBs own replicator.
The Apache CouchDB replicator itself handles `revpos:0` docs without a problem.
But the replication of those docs cannot be optimized; the attachments get
copied over again for every revision, even if they did not change.
@zgramana zgramana added this to the 1.3 milestone Apr 8, 2016
@snej snej added in progress and removed ready labels Apr 11, 2016
@snej snej self-assigned this Apr 11, 2016
@snej snej added review and removed in progress labels Apr 12, 2016
nolanlawson pushed a commit to pouchdb/pouchdb that referenced this issue Apr 14, 2016
Add the `revpos` property to attachment stubs on save. `revpos` is the doc's
`rev` prefix (a sequential number) from when the attachment was added or
updated. It is used during replication in the CouchDB replicator (and others
including CouchBase Lite) to avoid fetching attachments that have not changed
since last replication.

This solves a problem with attachments created in PouchDB, which are then
replicated to an Apache CouchDB instance. They end up having a `revpos` property
set to `0`. When the doc is later pull replicated from an iOS application using
CouchBase Lite it is skipped and does not get replicated. This is because CBL
rejects attachments with invalid attachment stub (see
couchbase/couchbase-lite-ios#1200). `revpos:0` is
considered invalid in CBL.

Also this improves replication performance of documents with attachments created
by PouchDB, when they are later replicated using Apache CouchDBs own replicator.
The Apache CouchDB replicator itself handles `revpos:0` docs without a problem.
But the replication of those docs cannot be optimized; the attachments get
copied over again for every revision, even if they did not change.
@pasin pasin closed this as completed Apr 19, 2016
@pasin pasin removed the review label Apr 19, 2016
@jo
Copy link

jo commented Apr 19, 2016

Thanks a lot!

@snej snej added bug and removed enhancement labels Jul 3, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants