Regards to taking lock in dictionary

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

Regards to taking lock in dictionary

Mohit Agrawal

I have a query why do we take a lock at the time of doing an operation in a dictionary.I have observed in testing it seems there is no codepath where
  we are using the dictionary parallel. In theory, the dictionary flow is like one xlator put some data in a dictionary and pass to the next xlator and xlator  and originator xlator won't touch dictionary until the called xlator returns.

  To prove the same I have executed below test case
  1) I have changed all LOCK/UNLOCK macro with dictlock/dictunlock function in the dictionary code and call the LOCK/UNLOCK macros
  2) Create a 1x3 volume and mount the volume
  3) Run stap script on one of the node to measure dict lock contention to log the entry if more than one thread access the dictionary at the same time
  4) Run smallfile tool like below with multiple operations like create/append/cleanup
     /root/sync.sh; python /root/smallfile/smallfile_cli.py --operation <op_name> --threads 8 --file-size 64 --files 5000 --top /mnt/test  --host-set "hp-m300-2.gsslab.pnq.redhat.com";
 
   I have not found any single thread that is trying to access the dictionary while dictlock is already held by some other thread.

  I have uploaded a patch(https://review.gluster.org/#/c/glusterfs/+/23603/) after converting the if condition to false in dictlock/unlock and run the 
  regression test suite.I am not getting major failures after removing the lock from a dictionary.

  Please share your view on the same if the dictionary is not consumed by multiple threads at the same time still we do need to take lock 
  in the dictionary.
  Please share if I need to test something more to validate the same.
  
Regards,
Mohit Agrawal

_______________________________________________

Community Meeting Calendar:

APAC Schedule -
Every 2nd and 4th Tuesday at 11:30 AM IST
Bridge: https://bluejeans.com/118564314

NA/EMEA Schedule -
Every 1st and 3rd Tuesday at 01:00 PM EDT
Bridge: https://bluejeans.com/118564314

Gluster-devel mailing list
[hidden email]
https://lists.gluster.org/mailman/listinfo/gluster-devel

Reply | Threaded
Open this post in threaded view
|

Re: Regards to taking lock in dictionary

Xavi Hernandez
Hi Mohit,

On Thu, Oct 24, 2019 at 5:19 AM Mohit Agrawal <[hidden email]> wrote:

I have a query why do we take a lock at the time of doing an operation in a dictionary.I have observed in testing it seems there is no codepath where
  we are using the dictionary parallel. In theory, the dictionary flow is like one xlator put some data in a dictionary and pass to the next xlator and xlator  and originator xlator won't touch dictionary until the called xlator returns.

  To prove the same I have executed below test case
  1) I have changed all LOCK/UNLOCK macro with dictlock/dictunlock function in the dictionary code and call the LOCK/UNLOCK macros
  2) Create a 1x3 volume and mount the volume
  3) Run stap script on one of the node to measure dict lock contention to log the entry if more than one thread access the dictionary at the same time
  4) Run smallfile tool like below with multiple operations like create/append/cleanup
     /root/sync.sh; python /root/smallfile/smallfile_cli.py --operation <op_name> --threads 8 --file-size 64 --files 5000 --top /mnt/test  --host-set "hp-m300-2.gsslab.pnq.redhat.com";
 
   I have not found any single thread that is trying to access the dictionary while dictlock is already held by some other thread.

  I have uploaded a patch(https://review.gluster.org/#/c/glusterfs/+/23603/) after converting the if condition to false in dictlock/unlock and run the 
  regression test suite.I am not getting major failures after removing the lock from a dictionary.

  Please share your view on the same if the dictionary is not consumed by multiple threads at the same time still we do need to take lock 
  in the dictionary.
  Please share if I need to test something more to validate the same.

That's very interesting. From a logical point of view I think that we shouldn't have two threads accessing the same dict at the same time. The only thing that locks guarantee is the internal integrity of the dict structure but, if we have concurrent access to the dict, one of the threads could see keys and/or values appearing/disappearing/changing spuriously, which surely could make things not work well. Apparently we don't have these issues.

If that's true, I think we should be able to completely get rid of the lock in dict structure.

This change is dangerous however, and it would need extensive testing. If any issue is found, probably we would need to fix that issue instead of adding locks again though.

Another important thing to do is to run some performance tests. If we really don't have contention in dict lock, the real cost of the lock is an atomic operation, which is not free but it's much cheaper than the kernel context that happens in case of contention. We would need to see the improvement to weight the benefits of this change.

Xavi


  
Regards,
Mohit Agrawal
_______________________________________________

Community Meeting Calendar:

APAC Schedule -
Every 2nd and 4th Tuesday at 11:30 AM IST
Bridge: https://bluejeans.com/118564314

NA/EMEA Schedule -
Every 1st and 3rd Tuesday at 01:00 PM EDT
Bridge: https://bluejeans.com/118564314

Gluster-devel mailing list
[hidden email]
https://lists.gluster.org/mailman/listinfo/gluster-devel


_______________________________________________

Community Meeting Calendar:

APAC Schedule -
Every 2nd and 4th Tuesday at 11:30 AM IST
Bridge: https://bluejeans.com/118564314

NA/EMEA Schedule -
Every 1st and 3rd Tuesday at 01:00 PM EDT
Bridge: https://bluejeans.com/118564314

Gluster-devel mailing list
[hidden email]
https://lists.gluster.org/mailman/listinfo/gluster-devel