Hi Mohit,
On Thu, Oct 24, 2019 at 5:19 AM Mohit Agrawal <moagrawa@xxxxxxxxxx> 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 theregression 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 lockin 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
Gluster-devel@xxxxxxxxxxx
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 Gluster-devel@xxxxxxxxxxx https://lists.gluster.org/mailman/listinfo/gluster-devel
