tests: Write the notification message to different files to avoid race condition in feature_notifications.py #14275

1. 
   ken2812221 commented at 8:27 am on September 20, 2018: contributor

   This PR change the behavior that feature_notifications.py would write to different files instead of writing to the same file to avoid race condition.
2. fanquake added the label Tests on Sep 20, 2018
3. 
   DrahtBot commented at 9:18 am on September 20, 2018: member

   • #14320 (wallet: Fix duplicate fileid detection by ken2812221)
   • #14316 (tests: exclude all tests with difference parameters in --exclude list by ken2812221)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

4. 
   MarcoFalke commented at 11:58 am on September 20, 2018: member

   utACK f8b6424a1014e8080f22b39a6d3b9998e85f2f90
5. 
   sdaftuar commented at 12:42 pm on September 20, 2018: member

   Could you explain the race condition?
6. 
   ken2812221 commented at 12:51 pm on September 20, 2018: contributor

   Could you explain the race condition?

   Bitcoin Core runs the each notification command in new thread, so it would have 10 process writing the same file if it generate 10 blocks immediately in this tests.

7. 
   MarcoFalke commented at 5:53 pm on September 20, 2018: member

   Maybe on a second thought the tests should mirror how the notifications are supposed to be used in practice. If we don’t support support filesystems that deny access when a lock is taken, as opposed to waiting, then the test shouldn’t run on that system.
8. 
   ryanofsky commented at 6:38 pm on September 20, 2018: member

   Instead of sleeping between the blocknotify commands, so they can all write to the same file, what about just having them write to different files. E.g. create a blocknotify/ directory and change -blocknotify=echo %s >> blocks.txt to -blocknotify=echo > blocknotify/%s
9. 
   promag commented at 10:34 pm on September 20, 2018: member

   @ryanofsky suggestion is good to fix the problem.

   But I wonder if we should replace the “call system with user command in a new thread” with a FIFO notification thread?

10. ken2812221 force-pushed on Sep 21, 2018
11. ken2812221 force-pushed on Sep 21, 2018
12. ken2812221 force-pushed on Sep 21, 2018
13. ken2812221 force-pushed on Sep 21, 2018
14. ken2812221 renamed this:
    tests: Slowly generate blocks to avoid race condition in feature_notifications.py
    tests: Write the notification message to different files to avoid race condition in feature_notifications.py
    on Sep 21, 2018
15. 
    ken2812221 commented at 6:36 am on September 21, 2018: contributor

    @ryanofsky Updated the commit. Now the test create empty files with different name.
16. in test/functional/feature_notifications.py:41 in 551b39a347 outdated

    38@@ -36,54 +39,49 @@ def run_test(self):
    39         blocks = self.nodes[1].generate(block_count)
    40 
    41         # wait at most 10 seconds for expected file size before reading the content
    

    ---

    ---

    

    sdaftuar commented at 7:08 pm on September 24, 2018:

    nit: comment should be updated: ...expected directory size...

    ---

    

    sdaftuar commented at 7:09 pm on September 24, 2018:

    also the comments below this one that refer to “file contents” should be “directory contents”
17. 
    sdaftuar commented at 7:50 pm on September 24, 2018: member

    The last commit looks good to me, modulo the comment nits. I have not reviewed the first two commits which are from #14007.
18. DrahtBot added the label Needs rebase on Sep 24, 2018
19. tests: write the notification to different files to avoid race condition 67654b6405
20. ken2812221 force-pushed on Sep 24, 2018
21. DrahtBot removed the label Needs rebase on Sep 24, 2018
22. 
    MarcoFalke commented at 5:24 pm on September 25, 2018: member

    utACK 67654b6405
23. MarcoFalke merged this on Sep 25, 2018
24. MarcoFalke closed this on Sep 25, 2018

    ---

25. MarcoFalke referenced this in commit bb8c9b3f74 on Sep 25, 2018
26. ken2812221 deleted the branch on Sep 25, 2018
27. deadalnix referenced this in commit 0011ed83a2 on May 6, 2020
28. ftrader referenced this in commit 5ac574978d on Aug 17, 2020
29. 5tefan referenced this in commit ded574b89a on Aug 13, 2021
30. 5tefan referenced this in commit ac3640899c on Aug 14, 2021
31. PastaPastaPasta referenced this in commit f370f41210 on Aug 16, 2021
32. MarcoFalke locked this on Sep 8, 2021

Contributors
ken2812221 DrahtBot MarcoFalke sdaftuar ryanofsky promag

Labels
Tests