PYTHON-1860 Use OP_MSG for find/aggregate_raw_batches when supported #622

prashantmital · 2021-05-13T17:37:11Z

No description provided.

ShaneHarvey

This PR seems outdated. @prashantmital can you push the new changes we made?

bson/__init__.py

ShaneHarvey · 2021-05-18T22:02:51Z

pymongo/message.py

+        codec_options = DEFAULT_CODEC_OPTIONS.with_options(
+            document_class=RawBSONDocument)
+        inflated_response = _decode_selective(
+            RawBSONDocument(self.payload_document), user_fields, codec_options)


Do we need this RawBSONDocument here?

I don't think we need _decode_selective either. I think we can just do:

inflated_response = raw_bson._inflate_bson(self.payload_document, DEFAULT_RAW_BSON_OPTIONS)

We do need _decode_selective because we need the top-level document that this returns to be a dictionary so that we can modify nextBatch/firstBatch in-place in the returned inflated_response. Else, we will need to perform a copy in _convert_raw_document_lists_to_streams to construct a dict.

Yes but I think raw_bson._inflate_bson does what we want here in a simpler way. Does it not?

pymongo/message.py

ShaneHarvey · 2021-05-18T22:25:28Z

Note these 3 test failures on "latest" are expected (I'm seeing them in other branches changes):

 [2021/05/18 22:04:57.302] FAIL [0.026s]: test_index_filter (test_collection.TestCollection)
 [2021/05/18 22:04:57.302] ----------------------------------------------------------------------
 [2021/05/18 22:04:57.302] Traceback (most recent call last):
 [2021/05/18 22:04:57.302]   File "/data/mci/9fc3560cd178f3fd98b33d747797bc89/src/test/__init__.py", line 486, in wrap
 [2021/05/18 22:04:57.302]     return f(*args, **kwargs)
 [2021/05/18 22:04:57.302]   File "/data/mci/9fc3560cd178f3fd98b33d747797bc89/src/test/test_collection.py", line 651, in test_index_filter
 [2021/05/18 22:04:57.302]     self.assertEqual("x_1", stage.get('indexName'))
 [2021/05/18 22:04:57.302] AssertionError: 'x_1' != None
 [2021/05/18 22:04:57.302] ======================================================================
 [2021/05/18 22:04:57.302] FAIL [0.016s]: test_command_monitoring_find_A_successful_find_event_with_a_getmore_and_the_server_kills_the_cursor (test_command_monitoring_spec.TestAllScenarios)
 [2021/05/18 22:04:57.302] ----------------------------------------------------------------------
 [2021/05/18 22:04:57.302] Traceback (most recent call last):
 [2021/05/18 22:04:57.302]   File "/data/mci/9fc3560cd178f3fd98b33d747797bc89/src/test/__init__.py", line 486, in wrap
 [2021/05/18 22:04:57.302]     return f(*args, **kwargs)
 [2021/05/18 22:04:57.302]   File "/data/mci/9fc3560cd178f3fd98b33d747797bc89/src/test/__init__.py", line 486, in wrap
 [2021/05/18 22:04:57.302]     return f(*args, **kwargs)
 [2021/05/18 22:04:57.302]   File "/data/mci/9fc3560cd178f3fd98b33d747797bc89/src/test/test_command_monitoring_spec.py", line 199, in run_scenario
 [2021/05/18 22:04:57.302]     self.assertEqual(val, actual[key])
 [2021/05/18 22:04:57.302] AssertionError: {u'ns': u'command-monitoring-tests.test', u'nextBatch': [{u'x': 44, u'_id': 4}], [truncated]... != {u'ns': u'command-monitoring-tests.test', u'nextBatch': [{u'x': 44, u'_id': 4}], [truncated]...
 [2021/05/18 22:04:57.302] - {u'id': 0L,
 [2021/05/18 22:04:57.302] ?         ^^
 [2021/05/18 22:04:57.302] + {u'id': 42,
 [2021/05/18 22:04:57.302] ?         ^^
 [2021/05/18 22:04:57.302]    u'nextBatch': [{u'_id': 4, u'x': 44}],
 [2021/05/18 22:04:57.302]    u'ns': u'command-monitoring-tests.test'}
 [2021/05/18 22:04:57.302] ======================================================================
 [2021/05/18 22:04:57.302] FAIL [0.041s]: test_A_successful_find_event_with_a_getmore_and_the_server_kills_the_cursor (test_unified_format.TestUnifiedTestFormatValidPassPocCommandMonitoring)
 [2021/05/18 22:04:57.302] ----------------------------------------------------------------------
 [2021/05/18 22:04:57.302] Traceback (most recent call last):
 [2021/05/18 22:04:57.302]   File "/data/mci/9fc3560cd178f3fd98b33d747797bc89/src/test/unified_format.py", line 970, in test_case
 [2021/05/18 22:04:57.302]     self.run_scenario(spec)
 [2021/05/18 22:04:57.302]   File "/data/mci/9fc3560cd178f3fd98b33d747797bc89/src/test/unified_format.py", line 957, in run_scenario
 [2021/05/18 22:04:57.302]     self.check_events(spec.get('expectEvents', []))
 [2021/05/18 22:04:57.302]   File "/data/mci/9fc3560cd178f3fd98b33d747797bc89/src/test/unified_format.py", line 910, in check_events
 [2021/05/18 22:04:57.302]     expected_event, listener.results[idx])
 [2021/05/18 22:04:57.302]   File "/data/mci/9fc3560cd178f3fd98b33d747797bc89/src/test/unified_format.py", line 510, in match_event
 [2021/05/18 22:04:57.302]     self.match_result(reply, actual.reply)
 [2021/05/18 22:04:57.302]   File "/data/mci/9fc3560cd178f3fd98b33d747797bc89/src/test/unified_format.py", line 463, in match_result
 [2021/05/18 22:04:57.302]     expectation, actual, is_root=not in_recursive_call)
 [2021/05/18 22:04:57.302]   File "/data/mci/9fc3560cd178f3fd98b33d747797bc89/src/test/unified_format.py", line 453, in _match_document
 [2021/05/18 22:04:57.302]     self.match_result(value, actual[key], in_recursive_call=True)
 [2021/05/18 22:04:57.302]   File "/data/mci/9fc3560cd178f3fd98b33d747797bc89/src/test/unified_format.py", line 463, in match_result
 [2021/05/18 22:04:57.302]     expectation, actual, is_root=not in_recursive_call)
 [2021/05/18 22:04:57.302]   File "/data/mci/9fc3560cd178f3fd98b33d747797bc89/src/test/unified_format.py", line 453, in _match_document
 [2021/05/18 22:04:57.302]     self.match_result(value, actual[key], in_recursive_call=True)
 [2021/05/18 22:04:57.302]   File "/data/mci/9fc3560cd178f3fd98b33d747797bc89/src/test/unified_format.py", line 478, in match_result
 [2021/05/18 22:04:57.302]     self._test_class.assertEqual(expectation, actual)
 [2021/05/18 22:04:57.302] AssertionError: 0 != 4400764155901009001L

The others I haven't seen before and might be caused by this PR:

 [2021/05/18 21:28:19.190] ERROR [0.014s]: test_read_concern (test_cursor.TestRawBatchCursor)
[2021/05/18 21:28:19.190] ----------------------------------------------------------------------
[2021/05/18 21:28:19.190] Traceback (most recent call last):
[2021/05/18 21:28:19.190]   File "/data/mci/c9dfda4ae77471eba031921ada430a9b/src/test/__init__.py", line 497, in wrap
[2021/05/18 21:28:19.190]     return f(*args, **kwargs)
[2021/05/18 21:28:19.190]   File "/data/mci/c9dfda4ae77471eba031921ada430a9b/src/test/test_cursor.py", line 1518, in test_read_concern
[2021/05/18 21:28:19.190]     next(c.find_raw_batches())
[2021/05/18 21:28:19.190]   File "/data/mci/c9dfda4ae77471eba031921ada430a9b/src/pymongo/cursor.py", line 1209, in next
[2021/05/18 21:28:19.190]     raise StopIteration
[2021/05/18 21:28:19.190] StopIteration

ShaneHarvey

LGTM. It would be nice to understand the perf impact of this change. Can you open a ticket for that?

prashantmital · 2021-05-19T19:05:28Z

See https://jira.mongodb.org/browse/PYTHON-2722

…622) (cherry picked from commit 209d500)

prashantmital requested a review from ShaneHarvey May 13, 2021 17:37

ShaneHarvey requested changes May 17, 2021

View reviewed changes

prashantmital added 3 commits May 17, 2021 21:12

somewhat functional prototype

127d6eb

test 1

27690ac

refactor

42d0b4b

prashantmital force-pushed the PYTHON-1860/dont-use-op-get-more branch from 06bac45 to 42d0b4b Compare May 18, 2021 04:31

get exhaust working

3276f8c

prashantmital marked this pull request as ready for review May 18, 2021 04:47

prashantmital requested a review from ShaneHarvey May 18, 2021 04:47

prashantmital added 2 commits May 18, 2021 13:09

fix rawbatchcommandcursor

54ef0af

fix empty cursor behavior

e8e747c

prashantmital changed the title ~~Python 1860/dont use op get more~~ PYTHON-1860 Use OP_MSG for find/aggregate_raw_batches when supported May 18, 2021

ShaneHarvey requested changes May 18, 2021

View reviewed changes

bson/__init__.py Outdated Show resolved Hide resolved

make code less generic

c392f58

prashantmital requested a review from ShaneHarvey May 18, 2021 21:04

ShaneHarvey requested changes May 18, 2021

View reviewed changes

prashantmital added 2 commits May 18, 2021 22:38

fix test failures

8ad0135

use WC majority to prevent failures

fc47f3d

ShaneHarvey approved these changes May 19, 2021

View reviewed changes

prashantmital merged commit 209d500 into mongodb:master May 19, 2021

prashantmital deleted the PYTHON-1860/dont-use-op-get-more branch May 19, 2021 19:05

prashantmital added a commit that referenced this pull request May 19, 2021

PYTHON-1860 Use OP_MSG for find/aggregate_raw_batches when supported (#…

0cbd7a2

…622) (cherry picked from commit 209d500)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PYTHON-1860 Use OP_MSG for find/aggregate_raw_batches when supported #622

PYTHON-1860 Use OP_MSG for find/aggregate_raw_batches when supported #622

prashantmital commented May 13, 2021

ShaneHarvey left a comment

ShaneHarvey May 18, 2021

ShaneHarvey May 18, 2021 •

edited

Loading

prashantmital May 19, 2021

ShaneHarvey May 19, 2021

ShaneHarvey commented May 18, 2021

ShaneHarvey left a comment

prashantmital commented May 19, 2021

PYTHON-1860 Use OP_MSG for find/aggregate_raw_batches when supported #622

PYTHON-1860 Use OP_MSG for find/aggregate_raw_batches when supported #622

Conversation

prashantmital commented May 13, 2021

ShaneHarvey left a comment

Choose a reason for hiding this comment

ShaneHarvey May 18, 2021

Choose a reason for hiding this comment

ShaneHarvey May 18, 2021 • edited Loading

Choose a reason for hiding this comment

prashantmital May 19, 2021

Choose a reason for hiding this comment

ShaneHarvey May 19, 2021

Choose a reason for hiding this comment

ShaneHarvey commented May 18, 2021

ShaneHarvey left a comment

Choose a reason for hiding this comment

prashantmital commented May 19, 2021

ShaneHarvey May 18, 2021 •

edited

Loading