[ 
https://issues.apache.org/jira/browse/BEAM-7866?focusedWorklogId=290120&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-290120
 ]

ASF GitHub Bot logged work on BEAM-7866:
----------------------------------------

                Author: ASF GitHub Bot
            Created on: 07/Aug/19 00:53
            Start Date: 07/Aug/19 00:53
    Worklog Time Spent: 10m 
      Work Description: chamikaramj commented on pull request #9233:  
[BEAM-7866] Fix python ReadFromMongoDB potential data loss issue
URL: https://github.com/apache/beam/pull/9233#discussion_r311330436
 
 

 ##########
 File path: sdks/python/apache_beam/io/mongodbio_test.py
 ##########
 @@ -30,38 +34,136 @@
 from apache_beam.io.mongodbio import _BoundedMongoSource
 from apache_beam.io.mongodbio import _GenerateObjectIdFn
 from apache_beam.io.mongodbio import _MongoSink
+from apache_beam.io.mongodbio import _ObjectIdHelper
+from apache_beam.io.mongodbio import _ObjectIdRangeTracker
 from apache_beam.io.mongodbio import _WriteMongoFn
 from apache_beam.testing.test_pipeline import TestPipeline
 from apache_beam.testing.util import assert_that
 from apache_beam.testing.util import equal_to
 
 
+class _MockMongoColl(object):
+  """Fake mongodb collection cursor."""
+
+  def __init__(self, docs):
+    self.docs = docs
+
+  def _filter(self, filter):
+    match = []
+    if not filter:
+      return self
+    start = filter['_id'].get('$gte')
+    end = filter['_id'].get('$lt')
+    assert start is not None
+    assert end is not None
+    for doc in self.docs:
+      if start and doc['_id'] < start:
+        continue
+      if end and doc['_id'] >= end:
+        continue
+      match.append(doc)
+    return match
+
+  def find(self, filter=None, **kwargs):
+    return _MockMongoColl(self._filter(filter))
+
+  def sort(self, sort_items):
+    key, order = sort_items[0]
+    self.docs = sorted(self.docs,
+                       key=lambda x: x[key],
+                       reverse=(order != ASCENDING))
+    return self
+
+  def limit(self, num):
+    return _MockMongoColl(self.docs[0:num])
+
+  def count_documents(self, filter):
+    return len(self._filter(filter))
+
+  def __getitem__(self, index):
+    return self.docs[index]
+
+
+class _MockMongoDb(object):
+  """Fake Mongo Db."""
+
+  def __init__(self, docs):
+    self.docs = docs
+
+  def __getitem__(self, coll_name):
+    return _MockMongoColl(self.docs)
+
+  def command(self, command, *args, **kwargs):
+    if command == 'collstats':
+      return {'size': 5, 'avgSize': 1}
+    elif command == 'splitVector':
+      return self.get_split_key(command, *args, **kwargs)
+
+  def get_split_key(self, command, ns, min, max, maxChunkSize, **kwargs):
+    # simulate mongo db splitVector command, return split keys base on chunk
+    # size, assuming every doc is of size 1mb
+    start_id = min['_id']
 
 Review comment:
   Prob. use different variable names since these override system functions.
 
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Issue Time Tracking
-------------------

    Worklog Id:     (was: 290120)
    Time Spent: 5h 20m  (was: 5h 10m)

> Python MongoDB IO performance and correctness issues
> ----------------------------------------------------
>
>                 Key: BEAM-7866
>                 URL: https://issues.apache.org/jira/browse/BEAM-7866
>             Project: Beam
>          Issue Type: Bug
>          Components: sdk-py-core
>            Reporter: Eugene Kirpichov
>            Assignee: Yichi Zhang
>            Priority: Blocker
>             Fix For: 2.15.0
>
>          Time Spent: 5h 20m
>  Remaining Estimate: 0h
>
> https://github.com/apache/beam/blob/master/sdks/python/apache_beam/io/mongodbio.py
>  splits the query result by computing number of results in constructor, and 
> then in each reader re-executing the whole query and getting an index 
> sub-range of those results.
> This is broken in several critical ways:
> - The order of query results returned by find() is not necessarily 
> deterministic, so the idea of index ranges on it is meaningless: each shard 
> may basically get random, possibly overlapping subsets of the total results
> - Even if you add order by `_id`, the database may be changing concurrently 
> to reading and splitting. E.g. if the database contained documents with ids 
> 10 20 30 40 50, and this was split into shards 0..2 and 3..5 (under the 
> assumption that these shards would contain respectively 10 20 30, and 40 50), 
> and then suppose shard 10 20 30 is read and then document 25 is inserted - 
> then the 3..5 shard will read 30 40 50, i.e. document 30 is duplicated and 
> document 25 is lost.
> - Every shard re-executes the query and skips the first start_offset items, 
> which in total is quadratic complexity
> - The query is first executed in the constructor in order to count results, 
> which 1) means the constructor can be super slow and 2) it won't work at all 
> if the database is unavailable at the time the pipeline is constructed (e.g. 
> if this is a template).
> Unfortunately, none of these issues are caught by SourceTestUtils: this class 
> has extensive coverage with it, and the tests pass. This is because the tests 
> return the same results in the same order. I don't know how to catch this 
> automatically, and I don't know how to catch the performance issue 
> automatically, but these would all be important follow-up items after the 
> actual fix.
> CC: [~chamikara] as reviewer.



--
This message was sent by Atlassian JIRA
(v7.6.14#76016)

Reply via email to