Skip to content
This repository has been archived by the owner on Dec 1, 2022. It is now read-only.

Improved data write logic to avoid reading dirty data #517

Merged
merged 3 commits into from
Jul 9, 2021
Merged

Improved data write logic to avoid reading dirty data #517

merged 3 commits into from
Jul 9, 2021

Conversation

bright-starry-sky
Copy link
Contributor

@bright-starry-sky bright-starry-sky commented Jul 6, 2021 •
edited
Loading

We are using ConcurrentHashMap to solve the performance issues of concurrent writes.
The current write logic may cause dirty data to be read, for example:
Thread A : -----> read -> lock -> write
Thread B : read----------------------------> lock -> write
If thread A terminates first, it will cause thread B to read dirty data.

changed to ：
Thread A : lock -----> read -> write -> unlock
Thread B : lock - > read-----------------------------> write -> unlock

@bright-starry-sky bright-starry-sky added the ready-for-testing PR: ready for the CI test label Jul 6, 2021
critical27
critical27 reviewed Jul 8, 2021
View reviewed changes
Copy link
Contributor

@critical27 critical27 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thx

liuyu85cn
liuyu85cn approved these changes Jul 8, 2021
View reviewed changes
Copy link
Contributor

@liuyu85cn liuyu85cn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if all these lock guard can be treated as member of processor. This will get rid of the unlock(..) things, and move lock guard to a lambda then UNUSED(l).
not important , LGTM

@bright-starry-sky
Copy link
Contributor Author

I'm not sure if all these lock guard can be treated as member of processor. This will get rid of the unlock(..) things, and move lock guard to a lambda then UNUSED(l).
not important , LGTM

Good idea, This is a good way to manage Gard Lock. let me think about that later and deal with everything together. but not in this PR. Thanks .

@bright-starry-sky bright-starry-sky merged commit 4e56a74 into vesoft-inc:master Jul 9, 2021
@bright-starry-sky bright-starry-sky deleted the fixed_concurrent_map branch July 9, 2021 01:49
@jamieliu1023 jamieliu1023 mentioned this pull request Jul 10, 2021
Closed
critical27 added a commit that referenced this pull request Jul 12, 2021
@critical27
Revert "Improved data write logic to avoid reading dirty data (#517)"
4b41d3b 
This reverts commit 4e56a74.
@critical27 critical27 mentioned this pull request Jul 12, 2021
Merged
critical27 added a commit that referenced this pull request Jul 12, 2021
@critical27
Revert "Improved data write logic to avoid reading dirty data (#517)" (…
ad4a36d 
…#521)

This reverts commit 4e56a74.
@bright-starry-sky
Copy link
Contributor Author

test error log:

======================================================================================= FAILURES =======================================================================================
__________________________________________________________________________ test_delete_string_vertex_by_pipe ___________________________________________________________________________
[gw0] linux -- Python 3.7.7 /usr/bin/python3
../../../../.local/lib/python3.7/site-packages/pytest_bdd/scenario.py:177: in scenario_wrapper
    _execute_scenario(feature, scenario, request, encoding)
../../../../.local/lib/python3.7/site-packages/pytest_bdd/scenario.py:143: in _execute_scenario
    _execute_step_function(request, scenario, step, step_func)
../../../../.local/lib/python3.7/site-packages/pytest_bdd/scenario.py:113: in _execute_step_function
    return_value = step_func(**kwargs)
tck/conftest.py:413: in execution_should_be_succ
    check_resp(rs, stmt)
common/utils.py:329: in check_resp
    assert resp.is_succeeded(), msg
E   AssertionError: Fail to exec: GO FROM "Boris Diaw" OVER like YIELD like._dst as id | DELETE VERTEX $-.id, error: Storage Error: More than one request trying to add/update/delete one edge/vertex at the same time.
--------------------------------------------------------------------------------- Captured stderr call ---------------------------------------------------------------------------------
[2021-07-12 12:32:08,758]:Get connection to ('127.0.0.1', 2138)
---------------------------------------------------------------------------------- Captured log call -----------------------------------------------------------------------------------
INFO     root:__init__.py:257 Get connection to ('127.0.0.1', 2138)
______________________________________________________________________________ test_delete_int_vid_vertex ______________________________________________________________________________
[gw3] linux -- Python 3.7.7 /usr/bin/python3
../../../../.local/lib/python3.7/site-packages/pytest_bdd/scenario.py:177: in scenario_wrapper
    _execute_scenario(feature, scenario, request, encoding)
../../../../.local/lib/python3.7/site-packages/pytest_bdd/scenario.py:143: in _execute_scenario
    _execute_step_function(request, scenario, step, step_func)
../../../../.local/lib/python3.7/site-packages/pytest_bdd/scenario.py:113: in _execute_step_function
    return_value = step_func(**kwargs)
tck/conftest.py:413: in execution_should_be_succ
    check_resp(rs, stmt)
common/utils.py:329: in check_resp
    assert resp.is_succeeded(), msg
E   AssertionError: Fail to exec: DELETE VERTEX hash("LeBron James"), hash("Dwyane Wade"), hash("Carmelo Anthony");, error: Storage Error: More than one request trying to add/update/delete one edge/vertex at the same time.
--------------------------------------------------------------------------------- Captured stderr call ---------------------------------------------------------------------------------
[2021-07-12 12:32:09,708]:Get connection to ('127.0.0.1', 2138)
---------------------------------------------------------------------------------- Captured log call -----------------------------------------------------------------------------------
INFO     root:__init__.py:257 Get connection to ('127.0.0.1', 2138)
_______________________________________________________________________ test_delete_int_vertex_by_pipe_successed _______________________________________________________________________
[gw4] linux -- Python 3.7.7 /usr/bin/python3
../../../../.local/lib/python3.7/site-packages/pytest_bdd/scenario.py:177: in scenario_wrapper
    _execute_scenario(feature, scenario, request, encoding)
../../../../.local/lib/python3.7/site-packages/pytest_bdd/scenario.py:143: in _execute_scenario
    _execute_step_function(request, scenario, step, step_func)
../../../../.local/lib/python3.7/site-packages/pytest_bdd/scenario.py:113: in _execute_step_function
    return_value = step_func(**kwargs)
tck/conftest.py:413: in execution_should_be_succ
    check_resp(rs, stmt)
common/utils.py:329: in check_resp
    assert resp.is_succeeded(), msg
E   AssertionError: Fail to exec: GO FROM hash("Boris Diaw") OVER like YIELD like._dst as id | DELETE VERTEX $-.id, error: Storage Error: More than one request trying to add/update/delete one edge/vertex at the same time.
--------------------------------------------------------------------------------- Captured stderr call ---------------------------------------------------------------------------------
[2021-07-12 12:32:09,776]:Get connection to ('127.0.0.1', 2138)
---------------------------------------------------------------------------------- Captured log call -----------------------------------------------------------------------------------
INFO     root:__init__.py:257 Get connection to ('127.0.0.1', 2138)
______________________________________________________________________________ test_delete_string_vertex _______________________________________________________________________________
[gw9] linux -- Python 3.7.7 /usr/bin/python3
../../../../.local/lib/python3.7/site-packages/pytest_bdd/scenario.py:177: in scenario_wrapper
    _execute_scenario(feature, scenario, request, encoding)
../../../../.local/lib/python3.7/site-packages/pytest_bdd/scenario.py:143: in _execute_scenario
    _execute_step_function(request, scenario, step, step_func)
../../../../.local/lib/python3.7/site-packages/pytest_bdd/scenario.py:113: in _execute_step_function
    return_value = step_func(**kwargs)
tck/conftest.py:413: in execution_should_be_succ
    check_resp(rs, stmt)
common/utils.py:329: in check_resp
    assert resp.is_succeeded(), msg
E   AssertionError: Fail to exec: DELETE VERTEX "LeBron James", "Dwyane Wade", "Carmelo Anthony";, error: Storage Error: More than one request trying to add/update/delete one edge/vertex at the same time.
--------------------------------------------------------------------------------- Captured stderr call ---------------------------------------------------------------------------------
[2021-07-12 12:32:10,172]:Get connection to ('127.0.0.1', 2138)
---------------------------------------------------------------------------------- Captured log call -----------------------------------------------------------------------------------
INFO     root:__init__.py:257 Get connection to ('127.0.0.1', 2138)
=============================================================================== short test summary info ================================================================================
FAILED tck/steps/test_tck.py::test_delete_string_vertex_by_pipe - AssertionError: Fail to exec: GO FROM "Boris Diaw" OVER like YIELD like._dst as id | DELETE VERTEX $-.id, error: St...
FAILED tck/steps/test_tck.py::test_delete_int_vid_vertex - AssertionError: Fail to exec: DELETE VERTEX hash("LeBron James"), hash("Dwyane Wade"), hash("Carmelo Anthony");, error: St...
FAILED tck/steps/test_tck.py::test_delete_int_vertex_by_pipe_successed - AssertionError: Fail to exec: GO FROM hash("Boris Diaw") OVER like YIELD like._dst as id | DELETE VERTEX $-....
FAILED tck/steps/test_tck.py::test_delete_string_vertex - AssertionError: Fail to exec: DELETE VERTEX "LeBron James", "Dwyane Wade", "Carmelo Anthony";, error: Storage Error: More t...
====================================================================== 4 failed, 1000 passed in 281.88s (0:04:41) ======================================================================

bright-starry-sky added a commit that referenced this pull request Jul 12, 2021
@bright-starry-sky
Revert "Improved data write logic to avoid reading dirty data (#517)"
d0489e1 
This reverts commit 4e56a74.
@bright-starry-sky bright-starry-sky mentioned this pull request Jul 12, 2021
Closed
bright-starry-sky added a commit that referenced this pull request Jul 12, 2021
@bright-starry-sky
Revert "Revert "Improved data write logic to avoid reading dirty data (…
6a475a8 
…#517)" (#521)"

This reverts commit ad4a36d.
bright-starry-sky added a commit that referenced this pull request Jul 12, 2021
@bright-starry-sky
re-submit "Improved data write logic to avoid reading dirty data" (#525)
b848eea 
* Revert "Revert "Improved data write logic to avoid reading dirty data (#517)" (#521)"

This reverts commit ad4a36d.

* dedup for delete vertex and edge
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@panda-sheep panda-sheep panda-sheep left review comments

@liuyu85cn liuyu85cn liuyu85cn approved these changes

@critical27 critical27 critical27 approved these changes

@darionyaphet darionyaphet Awaiting requested review from darionyaphet

Assignees
No one assigned
Labels
ready-for-testing PR: ready for the CI test
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@bright-starry-sky @critical27 @liuyu85cn @panda-sheep