summaryrefslogtreecommitdiffstats
path: root/xlators/cluster
Commit message (Collapse)AuthorAgeFilesLines
* afr: assign gfid during name heal when no 'source' is present.Ravishankar N2018-12-124-52/+52
| | | | | | | | | | | | | | | | | | | Problem: If parent dir is in split-brain or has dirty xattrs set, and the file has gfid missing on one of the bricks, then name heal won't assign the gfid. Fix: Use the brick we select the gfid from as the 'source'. Note: Problem was found while trying to debug a split-brain issue on Cynthia Zhou's setup. fixes: bz#1655545 Change-Id: Id088d4f0fb017aa35122de426654194e581ed742 Reported-by: Cynthia Zhou <cynthia.zhou@nokia-sbell.com> Signed-off-by: Ravishankar N <ravishankar@redhat.com> (cherry picked from commit 4d58730c0cd6ab5db39aec8a15276f7bd3371b04)
* afr: open_ftruncate_cbk should read fd from local->cont.open structSoumya Koduri2018-11-291-2/+2
| | | | | | | | | | | | | afr_open stores the fd as part of its local->cont.open struct but when it calls ftruncate (if open flags contain O_TRUNC), the corresponding cbk function (afr_ open_ftruncate_cbk) is incorrectly referencing uninitialized local->fd. This patch fixes the same. Change-Id: Icbdedbd1b8cfea11d8f41b6e5c4cb4b44d989aba updates: bz#1651322 Signed-off-by: Soumya Koduri <skoduri@redhat.com> (cherry picked from commit fda594875c4cdb2a22e27aa13f5c66bee032ccb5)
* cluster/afr: Use 2 domain locking in SHD for thin-arbiterkarthik-us2018-11-293-89/+160
| | | | | | | | | | | | | | | | | | | | | | | | With this change when SHD starts the index crawl it requests all the clients to release the AFR_TA_DOM_NOTIFY lock so that clients will know the in memory state is no more valid and any new operations needs to query the thin-arbiter if required. When SHD completes healing all the files without any failure, it will again take the AFR_TA_DOM_NOTIFY lock and gets the xattrs on TA to see whether there are any new failures happened by that time. If there are new failures marked on TA, SHD will start the crawl immediately to heal those failures as well. If there are no new failures, then SHD will take the AFR_TA_DOM_MODIFY lock and unsets the xattrs on TA, so that both the data bricks will be considered as good there after. >Change-Id: I037b89a0823648f314580ba0716d877bd5ddb1f1 >fixes: bz#1579788 >Signed-off-by: karthik-us <ksubrahm@redhat.com> (cherry picked from commit 5784a00f997212d34bd52b2303e20c097240d91c) Change-Id: I037b89a0823648f314580ba0716d877bd5ddb1f1 fixes: bz#1648205
* cluster/ec: prevent infinite loop in self-heal fullXavi Hernandez2018-11-291-5/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | There was a problem in commit 7f81067 that caused infinite loop when full heal was triggered. The previous commit was made to prevent self-heal to go idle after a replace brick operation. One of the changes consisted on setting a flag to force an immediate scan of the dirty directory if a heal on a directory succeeded (assuming it could have generated newer entries). However that change was causing an issue with a full self-heal, since every time an already healed directory was checked and it returned suceessfully, it was also setting the flag, forcing self-heal to start over again. This patch fixes this issue by only setting the flag if the heal is not full. It's assumed that a full self-heal will already traverse all entries automatically, so there's no need to force a new scan later. >Change-Id: Id12dbfc04e622b18183e796cc6cc87ccc30a6d55 >fixes: bz#1636631 >Signed-off-by: Xavi Hernandez <xhernandez@redhat.com> (cherry picked from commit 7150c51ad75ccba22045a35fc31e5037612d1ad4) Change-Id: Id12dbfc04e622b18183e796cc6cc87ccc30a6d55 fixes: bz#1651525 Signed-off-by: Xavi Hernandez <xhernandez@redhat.com>
* cluster/ec: Change log level to DEBUG for lookup combineAshish Pandey2018-11-141-1/+1
| | | | | | | | | | | | | | As lookup is not a locked fop, we can not trust the data received in this to be same. Changing the log level to DEBUG in case lookup finds any difference. (cherry picked from commit 9be6bf3d90e3783b3ba559c93d41b933f8d53f03) Change-Id: I39499c44688a2455c7c6c69a798762d045d21b39 updates: bz#1644622 Signed-off-by: Ashish Pandey <aspandey@redhat.com>
* all: fix the format string exceptionsAmar Tumballi2018-11-094-13/+14
| | | | | | | | | | | | | | | | Currently, there are possibilities in few places, where a user-controlled (like filename, program parameter etc) string can be passed as 'fmt' for printf(), which can lead to segfault, if the user's string contains '%s', '%d' in it. While fixing it, makes sense to make the explicit check for such issues across the codebase, by making the format call properly. Fixes: CVE-2018-14661 Fixes: bz#1647666 Change-Id: Ib547293f2d9eb618594cbff0df3b9c800e88bde4 Signed-off-by: Amar Tumballi <amarts@redhat.com>
* cluster/afr : Check for UP bricks before starting healAshish Pandey2018-11-083-1/+19
| | | | | | | | | | | | | | | | | | | | | | | | | Problem: Currently for replica volume, even if only one brick is UP SHD will keep crawling index entries even if it can not heal anything. In thin-arbiter volume which is also a replica 2 volume, this causes inode lock contention which in turn sends upcall to all the clients to release notify locks, even if it can not do anything for healing. This will slow down the client performance and kills the purpose of keeping in memory information about bad brick. Solution: Before starting heal or even crawling, check if sufficient number of children are UP and available to check and heal entries. (cherry picked from commit f73b4476b15f9d6d3dc3c8e20c9742aacd857f9f) Change-Id: I011c9da3b37cae275f791affd56b8f1c1ac9255d updates: bz#1644645 Signed-off-by: Ashish Pandey <aspandey@redhat.com>
* afr/lease: Read child nodes from lease structureroot2018-11-081-1/+1
| | | | | | | | | | For lease operation, we allocate and store child nodes data in lease structure. Use the same in afr_lease_cbk() while checking for the quorum. Change-Id: If1fdd5a0798888afd39ad3df57d96487baf9d1e6 updates: #350 Signed-off-by: Soumya Koduri <skoduri@redhat.com>
* afr: fix incorrect reporting of directory split-brainRavishankar N2018-10-117-16/+22
| | | | | | | | | | | | | | | | | | | Backport of https://review.gluster.org/#/c/glusterfs/+/21135/ Problem: When a directory has dirty xattrs due to failed post-ops or when replace/reset brick is performed, AFR does a conservative merge as expected, but heal-info reports it as split-brain because there are no clear sources. Fix: Modify pending flag to contain information about pending heals and split-brains. For directories, if spit-brain flag is not set,just show them as needing heal and not being in split-brain. Change-Id: I09ef821f6887c87d315ae99e6b1de05103cd9383 fixes: bz#1638163 Signed-off-by: Ravishankar N <ravishankar@redhat.com>
* afr: prevent winding inodelks twice for arbiter volumesRavishankar N2018-10-111-1/+1
| | | | | | | | | | | | | | | | | | | | Backport of https://review.gluster.org/#/c/glusterfs/+/21380/ Problem: In an arbiter volume, if there is a pending data heal of a file only on arbiter brick, self-heal takes inodelks twice due to a code-bug but unlocks it only once, leaving behind a stale lock on the brick. This causes the next write to the file to hang. Fix: Fix the code-bug to take lock only once. This bug was introduced master with commit eb472d82a083883335bc494b87ea175ac43471ff Thanks to Pranith Kumar K <pkarampu@redhat.com> for finding the RCA. fixes: bz#1638159 Change-Id: I15ad969e10a6a3c4bd255e2948b6be6dcddc61e1 Signed-off-by: Ravishankar N <ravishankar@redhat.com>
* cluster/afr: Make data eager-lock decision based on number of locksPranith Kumar K2018-10-053-6/+47
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | For both Virt and block workloads the file is opened multiple times leading to dynamically setting eager-lock to off for the workload. Instead of depending on the number-of-open-fds, if we change the logic to depend on number of inodelks, then it will give better performance than the earlier logic. When there is an eager-lock and number of inodelks is more than 1 we know that there is a conflicting lock, so depend on that information to decide whether to keep the current transaction go through delayed-post-op or not. Locks xlator doesn't have implementation to query number of locks in fxattrop in releases older than 3.10 so to keep things backward compatible in 3.12, data transactions will use new logic where as fxattrop transactions will use old logic. I am planning to send one more patch which makes metadata domain locks also depend on inodelk-count Profile info for a dd of 500MB to a file with another fd opened on the file using exec 250>filename Without this patch: 0.14 67.41 us 16.72 us 3870.82 us 892 FINODELK 0.59 279.87 us 95.71 us 2085.89 us 898 FXATTROP 3.46 366.43 us 81.75 us 6952.79 us 4000 WRITE 95.79 148733.99 us 50568.12 us 919127.86 us 273 FSYNC With this patch: 0.00 51.01 us 38.07 us 80.16 us 4 FINODELK 0.00 235.43 us 235.43 us 235.43 us 1 TRUNCATE 0.00 125.07 us 56.80 us 193.33 us 2 GETXATTR 0.00 135.86 us 62.13 us 209.59 us 2 INODELK 0.00 197.88 us 155.39 us 253.90 us 4 FXATTROP 0.00 450.59 us 394.28 us 506.89 us 2 XATTROP 0.00 56.96 us 19.06 us 406.59 us 23 FLUSH 37.81 273648.93 us 48.43 us 6017657.05 us 44 LOOKUP 62.18 4951.86 us 93.80 us 1143154.75 us 3999 WRITE postgresql benchmark performance changed from ~1130 TPS to ~2300TPS randio fio job inside Ovirt based VM went from ~600IOPs to ~2000IOPS fixes bz#1635972 Change-Id: If7f7388d2f08cf7f17ca517a4ea222560661dc36 Signed-off-by: Pranith Kumar K <pkarampu@redhat.com>
* cluster/afr: Batch writes in same lock even when multiple fds are openPranith Kumar K2018-10-051-9/+2
| | | | | | | | | | | | | | | | | | | | | | Problem: When eager-lock is disabled because of multiple-fds opened and app writes come on conflicting regions, the number of locks grows very fast leading to all the CPU being spent just in locking and unlocking by traversing huge queues in locks xlator for granting locks. Fix: Reduce the number of locks in transit by bundling the writes in the same lock and disable delayed piggy-pack when we learn that multiple fds are open on the file. This will reduce the size of queues in the locks xlator. This also reduces the number of network calls like inodelk/fxattrop. Please note that this problem can still happen if eager-lock is disabled as the writes will not be bundled in the same lock. fixes bz#1635975 Change-Id: I8fd1cf229aed54ce5abd4e6226351a039924dd91 Signed-off-by: Pranith Kumar K <pkarampu@redhat.com>
* gfapi: revert several patchs that introduced pre/post attrsShyamsundarR2018-09-172-13/+12
| | | | | | | | | | | | | | Reverted the following: - 248152767b0599986bbb6bb35fc27197f6be6964 - 09943beb499617212f2985ca8ea9ecd1ed1b470e - d01f7244e9d9f7e3ef84e0ba7b48ef1b1b09d809 The reverts are redone by hand, due to clang format changes that made using git to revert the changes more tedious. Change-Id: I96489638a2b641fb2206a110298543225783f7be Updates: bz#1628620 Signed-off-by: ShyamsundarR <srangana@redhat.com>
* build: cleanup xlator link, --no-undefined, libuuidv6devKaleb S. KEITHLEY2018-09-121-8/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | While attempting to build a (pre-)5.0 of glusterfs on Ubuntu bionic and cosmic, it became apparent that there are some gremlins hiding in the combination of the xlator export-symbols, the newish addition of -Wl,--no-undefined, and the new switch to libuuid from the old contrib/uuid. Note: even though Fedora 28 (and later) and Ubuntu bionic (and later) have the same nominal version of libtool, the Fedora version appears to do a better job of recursing through dependencies to determine the libraries to link with. Examination of the build logs showed that despite appearing to work on Fedora, not all xlators and shared libs were linked with -Wl, --no-undefined, and -luuid. And in the case of the gnfs xlator, it was not only not linked with -Wl,--no-undefined but alsos not linked with -lgfxdr and -lgfrpc. Added GF_XLATOR_LDFLAGS, similar to GF_XLATOR_DEFAULT_LDFLAGS. GF_XLATOR_DEFAULT_LDFLAGS is for xlators that export/expose the default or common set of symbols. GF_XLATOR_LDFLAGS is for those remaining xlators that export/expose non-default symbols, e.g. dht and glupy. This removes the need in the future to add things like $(UUID_LIBS) to every xlator's Makefile.am. Just add it to GF_XLATOR_LDFLAGS and GF_XLATOR_DEFAULT_LDFLAGS in configure.ac and you're done. This patch was tested on Fedora 28 (build, rpmbuild), Fedora Rawhide/30 (rpmbuild), RHEL8 (rpmbuild), CentOS7 (rpmbuild), Fedora koji --scratch build for f30/rawhide, and a Launchpad build for Ubuntu cosmic/18.10. Change-Id: Ieca104fa5c5d3c094e701c8ca4a73754dd0292b0 updates: bz#1193929 Signed-off-by: Kaleb S. KEITHLEY <kkeithle@redhat.com>
* Land part 2 of clang-format changesGluster Ant2018-09-1259-70721/+67551
| | | | | Change-Id: Ia84cc24c8924e6d22d02ac15f611c10e26db99b4 Signed-off-by: Nigel Babu <nigelb@redhat.com>
* Land clang-format changesGluster Ant2018-09-1232-3929/+3873
| | | | Change-Id: I6f5d8140a06f3c1b2d196849299f8d483028d33b
* dht: Use snprintf instead of strncpyN Balachandran2018-09-121-9/+20
| | | | | | | | | | | The recent changes to use malloc instead of calloc left the new_name and new_path non-null terminated. We now use snprintf instead of strncpy to fix this. Change-Id: I1a31701ca9447efde38921be0ba2c73cde2e7976 fixes: bz#1626346 Signed-off-by: N Balachandran <nbalacha@redhat.com>
* cluster/dht: Create a linkto file if requiredN Balachandran2018-09-101-0/+30
| | | | | | | | | | | Using the dht_filter_loc_subvol_key to create files on specific subvols did not create a linkto file. This can make the file inaccessible as lookup-optimize is now enabled by default. Change-Id: I78add5a31887378a479cb9c746b91678876b0dbe fixes: bz#1626394 Signed-off-by: N Balachandran <nbalacha@redhat.com>
* cluster/dht: optimize readdir for 1xn volsN Balachandran2018-09-101-12/+42
| | | | | | | | | Skip the hashed subvol check for volumes with distribute count of 1. Change-Id: I5703508b54a17c49a217c8a8e09884980705953a fixes: bz#1608175 Signed-off-by: N Balachandran <nbalacha@redhat.com>
* ec-heal: remove a duplicate definition of alloca0Amar Tumballi2018-09-101-1/+0
| | | | | | | | | | the same macro is defined in common-utils.h, which seems to be much better place for the same. Updates: bz#1193929 Change-Id: I409b719c291102136500b955e5827a550142ed96 Signed-off-by: Amar Tumballi <amarts@redhat.com>
* xlators/cluster/dht/src/tier-common.c:move to GF_MALLOC() instead of ↵Yaniv Kaul2018-09-101-1/+1
| | | | | | | | | | | | | | | | | | GF_CALLOC() when It doesn't make sense to calloc (allocate and clear) memory when the code right away fills that memory with data. It may be optimized by the compiler, or have a microscopic performance improvement. Please review carefully, especially for string allocation, with the terminating NULL string. Only compile-tested! Change-Id: I7d38a7d576f6777976fe86e5351a8d95caddbb9c updates: bz#1193929 Signed-off-by: Yaniv Kaul <ykaul@redhat.com>
* cluster/dht: Rework the debug xattr to get hashed subvolN Balachandran2018-09-072-28/+33
| | | | | | | | | | | | | | | | | | | The earlier implementation required the file to already exist when trying to get the hashed subvol. The reworked implementation allows a user to get the hashed subvol for any filename, whether it exists or not. Usage: getfattr -n "dht.file.hashed-subvol.<filename>" <parent dir> Eg:To get the hashed subvol for file-1 inside dir-1 getfattr -n "dht.file.hashed-subvol.file-1" /mnt/gluster/dir1 credit: rgowdapp@redhat.com Change-Id: Iae20bd5f56d387ef48c1c0a4ffa9f692866bf739 fixes: bz#1624244 Signed-off-by: N Balachandran <nbalacha@redhat.com>
* cluster/ec: Don't update trusted.ec.version if fop succeedsAshish Pandey2018-09-071-0/+9
| | | | | | | | | | | | | | | | | | | | If a fop has succeeded on all the bricks and trying to release the lock, there is no need to update the version for the file/entry. All it will do is to increase the version from x to x+1 on all the bricks. If this update (x to x+1) fails on some brick, this will indicate that the entry is unhealthy while in realty everything is fine with the entry. Avoiding this update will help to not to send one xattrop at the end of the fops. Which will decrease the chances of entries being in unhealthy state and also improve the performance. Change-Id: Id9fca6bd2991425db6ed7d1f36af27027accb636 fixes: bz#1623759 Signed-off-by: Ashish Pandey <aspandey@redhat.com>
* cluster/ec: Improve logging for some critical error messagesAshish Pandey2018-09-073-14/+55
| | | | | | Change-Id: I037e52a3467467b81a1ba5416317870864060d4d updates: bz#1615703 Signed-off-by: Ashish Pandey <aspandey@redhat.com>
* xlators/cluster/dht/src/dht-common.c :move to GF_MALLOC() instead of ↵Yaniv Kaul2018-09-071-2/+2
| | | | | | | | | | | | | | | | | | GF_CALLOC() when It doesn't make sense to calloc (allocate and clear) memory when the code right away fills that memory with data. It may be optimized by the compiler, or have a microscopic performance improvement. Please review carefully, especially for string allocation, with the terminating NULL string (added another byte to ensure it's there). Only compile-tested! Change-Id: Ia5e4f50dfb0c29809c2019fcfd8079507813249e updates: bz#1193929 Signed-off-by: Yaniv Kaul <ykaul@redhat.com>
* multiple xlators: strncpy()->sprintf(), reduce strlen()'sYaniv Kaul2018-09-074-33/+30
| | | | | | | | | | | | | | | | | | | | | | | xlators/cluster/afr/src/afr-common.c xlators/cluster/dht/src/dht-common.c xlators/cluster/dht/src/dht-rebalance.c xlators/cluster/stripe/src/stripe-helpers.c strncpy may not be very efficient for short strings copied into a large buffer: If the length of src is less than n, strncpy() writes additional null bytes to dest to ensure that a total of n bytes are written. Instead, use snprintf(). Also: - save the result of strlen() and re-use it when possible. - move from strlen to SLEN (sizeof() ) for const strings. Compile-tested only! Change-Id: Icdf79dd3d9f9ff120e4720ff2b8bd016df575c38 updates: bz#1193929 Signed-off-by: Yaniv Kaul <ykaul@redhat.com>
* cluster/dht: Initialise pointers to nullN Balachandran2018-09-061-2/+2
| | | | | | | | | | | | | | | | Use calloc in dht_layouts_init so to as to prevent dht_init from attempting to free invalid memory in case of failure. There are other ways to do this (set first failure to null and break there when cleaning up) but I prefer having all pointers initialized to null. This is a one time operation so it should not be too expensive. Change-Id: Ie22246047448f1cae971d48fa5aaf2efcaeb42c0 fixes: bz#1625643 Signed-off-by: N Balachandran <nbalacha@redhat.com>
* afr: thin-arbiter read txn changesRavishankar N2018-09-053-19/+255
| | | | | | | | | | | | | | | | If both data bricks are up, read subvol will be based on read_subvols. If only one data brick is up: - First qeury the data-brick that is up. If it blames the other brick, allow the reads. - If if doesn't, query the TA to obtain the source of truth. TODO: See if in-memory state can be maintained for read txns (BZ 1624358). updates: bz#1579788 Change-Id: I61eec35592af3a1aaf9f90846d9a358b2e4b2fcc Signed-off-by: Ravishankar N <ravishankar@redhat.com>
* glusterd: Fix Buffer size issuesSanju Rakonde2018-09-041-3/+3
| | | | | | | | This patch fixes buffer size issue 1138522. Change-Id: Ia12fc8f34f75704f8ed3efae2022c4fd67a8c76c updates: bz#789278 Signed-off-by: Sanju Rakonde <srakonde@redhat.com>
* {dht-rebalance|glusterd-geo-rep|glusterd-utils|nfs|bd}.c: no dict_del before ↵Yaniv Kaul2018-09-041-1/+0
| | | | | | | | | | | | dict_set There is no need to remove an item before re-setting it. Compile-tested only! Change-Id: I2869aec9ebf474859127b8b38d284246e6097e84 updates: bz#1193929 Signed-off-by: Yaniv Kaul <ykaul@redhat.com>
* multiple files: calloc -> mallocYaniv Kaul2018-09-047-41/+50
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | xlators/cluster/stripe/src/stripe-helpers.c: Move to GF_MALLOC() instead of GF_CALLOC() when possible xlators/cluster/dht/src/tier.c: Move to GF_MALLOC() instead of GF_CALLOC() when possible xlators/cluster/dht/src/dht-layout.c: Move to GF_MALLOC() instead of GF_CALLOC() when possible xlators/cluster/dht/src/dht-helper.c: Move to GF_MALLOC() instead of GF_CALLOC() when possible xlators/cluster/dht/src/dht-common.c: Move to GF_MALLOC() instead of GF_CALLOC() when possible xlators/cluster/afr/src/afr.c: Move to GF_MALLOC() instead of GF_CALLOC() when possible xlators/cluster/afr/src/afr-inode-read.c: Move to GF_MALLOC() instead of GF_CALLOC() when possible tests/bugs/replicate/bug-1250170-fsync.c: Move to GF_MALLOC() instead of GF_CALLOC() when possible tests/basic/gfapi/gfapi-async-calls-test.c: Move to GF_MALLOC() instead of GF_CALLOC() when possible tests/basic/ec/ec-fast-fgetxattr.c: Move to GF_MALLOC() instead of GF_CALLOC() when possible rpc/xdr/src/glusterfs3.h: Move to GF_MALLOC() instead of GF_CALLOC() when possible rpc/rpc-transport/socket/src/socket.c: Move to GF_MALLOC() instead of GF_CALLOC() when possible rpc/rpc-lib/src/rpc-clnt.c: Move to GF_MALLOC() instead of GF_CALLOC() when possible extras/geo-rep/gsync-sync-gfid.c: Move to GF_MALLOC() instead of GF_CALLOC() when possible cli/src/cli-xml-output.c: Move to GF_MALLOC() instead of GF_CALLOC() when possible cli/src/cli-rpc-ops.c: Move to GF_MALLOC() instead of GF_CALLOC() when possible cli/src/cli-cmd-volume.c: Move to GF_MALLOC() instead of GF_CALLOC() when possible cli/src/cli-cmd-system.c: Move to GF_MALLOC() instead of GF_CALLOC() when possible cli/src/cli-cmd-snapshot.c: Move to GF_MALLOC() instead of GF_CALLOC() when possible cli/src/cli-cmd-peer.c: Move to GF_MALLOC() instead of GF_CALLOC() when possible cli/src/cli-cmd-global.c: Move to GF_MALLOC() instead of GF_CALLOC() when possible It doesn't make sense to calloc (allocate and clear) memory when the code right away fills that memory with data. It may be optimized by the compiler, or have a microscopic performance improvement. In some cases, also changed allocation size to be sizeof some struct or type instead of a pointer - easier to read. In some cases, removed redundant strlen() calls by saving the result into a variable. 1. Only done for the straightforward cases. There's room for improvement. 2. Please review carefully, especially for string allocation, with the terminating NULL string. Only compile-tested! updates: bz#1193929 Original-Author: Yaniv Kaul <ykaul@redhat.com> Signed-off-by: Yaniv Kaul <ykaul@redhat.com> Signed-off-by: Amar Tumballi <amarts@redhat.com> Change-Id: I16274dca4078a1d06ae09a0daf027d734b631ac2
* xlators/cluster/afr/src/afr-inode-read.c: move to GF_MALLOC() instead of ↵Yaniv Kaul2018-09-041-1/+1
| | | | | | | | | | | | | | | | | | GF_CALLOC() when It doesn't make sense to calloc (allocate and clear) memory when the code right away fills that memory with data. It may be optimized by the compiler, or have a microscopic performance improvement. Please review carefully, especially for string allocation, with the terminating NULL string. Only compile-tested! Change-Id: Ief156de98769fea852553044a398a309e831754b updates: bz#1193929 Signed-off-by: Yaniv Kaul <ykaul@redhat.com>
* cluster/afr: Delegate name-heal when possiblePranith Kumar K2018-09-042-27/+85
| | | | | | | | | | | | | | | | | | | | | | | | | Problem: When name-self-heal is triggered on the mount, it blocks lookup until name-self-heal completes. But that can lead to hangs when lot of clients are accessing a directory which needs name heal and all of them trigger heals waiting for other clients to complete heal. Fix: When a name-heal is needed but quorum number of names have the file and pending xattrs exist on the parent, then better to delegate the heal to SHD which will be completed as part of entry-heal of the parent directory. We could also do the same for quorum-number of names not present but we don't have any known use-case where this is a frequent occurrence so not changing that part at the moment. When there is a gfid mismatch or missing gfid it is important to complete the heal so that next rename doesn't assume everything is fine and perform a rename etc fixes bz#1622821 Change-Id: I8b002c85dffc6eb6f2833e742684a233daefeb2c Signed-off-by: Pranith Kumar K <pkarampu@redhat.com>
* cluster/dht: In rename, unlink after creating linkto fileN Balachandran2018-09-031-122/+132
| | | | | | | | | | | | | | The linkto file creation for the dst was done in parallel with the unlink of the old src linkto. If these operations reached the brick out of order, we end up with a dst linkto file without a .glusterfs handle. Fixed by the unlinking only after the linkto file creation has completed. Change-Id: I4246f7655f5bc180f5ded7fd34d263b7828a8110 fixes: bz#1621981 Signed-off-by: N Balachandran <nbalacha@redhat.com>
* cluster/ec: Fix Coverity issueAshish Pandey2018-08-312-42/+0
| | | | | | | | | | | | | | | | | | | | | | | | Fix following coverity issues- CID: 1382378 1382459 https://scan6.coverity.com/reports.htm#v42607/p10714/fileInstanceId=85091670&defectInstanceId=25915064&mergedDefectId=1382459 https://scan6.coverity.com/reports.htm#v42607/p10714/fileInstanceId=85091670&defectInstanceId=25915063&mergedDefectId=1382378 Problem: ASSERT_LOCAL(this, healer) function is supposed to get the local healer so that we can take advantage of it while healing and reading data. However, we are not using healer->local anywhere. Also, this is not as useful in context of EC as it is in AFR. In EC we have to raed fragments from 4 bricks to heal a bad fragment on other brick. Change-Id: Iea8ce127ea02cc84e3823cb2be82a47872217b33 updates: bz#789278 Signed-off-by: Ashish Pandey <aspandey@redhat.com>
* xlators/cluster/dht/src/dht-common.c: simplify some if statementsYaniv Kaul2018-08-311-48/+52
| | | | | | | | | | | | Use goto when some vars are not set to simplify long if statements. Please review logic has not changed! Compile-tested only! updates: bz#1193929 Signed-off-by: Yaniv Kaul <ykaul@redhat.com> Change-Id: I45ea2e906d0ccb468af5e1fa65db008edb00d734
* build: add --enable-asan configure optionsNiels de Vos2018-08-301-1/+1
| | | | | | | | | | | | | | Introduce a `./configure --enable-asan` to build with `-fsanitize=address -fno-omit-frame-pointer` options. This uses the libasan.so shared library, so that needs to be available. While running builds with the ASAN options, several linker issues surfaced and these have been addressed with this change as well. Building with --enable-asan has been tested on Fedora 28. Change-Id: I428a9da70dd8f7d0056cfbe5c398619a571469b2 Updates: #492 Signed-off-by: Niels de Vos <ndevos@redhat.com>
* multiple files: remove unndeeded memset()Yaniv Kaul2018-08-293-4/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This is a squash of multiple commits: contrib/fuse-lib/misc.c: remove unneeded memset() All flock variables are properly set, no need to memset it. Only compile-tested! Signed-off-by: Yaniv Kaul <ykaul@redhat.com> Change-Id: I8e0512c5a88daadb0e587f545fdb9b32ca8858a2 libglusterfs/src/{client_t|fd|inode|stack}.c: remove some memset() I don't think there's a need for any of them. Only compile-tested! Signed-off-by: Yaniv Kaul <ykaul@redhat.com> Change-Id: I2be9ccc3a5cb5da51a92af73488cdabd1c527f59 libglusterfs/src/xlator.c: remove unneeded memset() All xl->mem_acct members are properly set, no need to memset it. Only compile-tested! Signed-off-by: Yaniv Kaul <ykaul@redhat.com> Change-Id: I7f264cd47e7a06255a3f3943c583de77ae8e3147 xlators/cluster/afr/src/afr-self-heal-common.c: remove unneeded memset() Since we are going over the whole array anyway, initialize it properly, to either 1 or 0. Only compile-tested! Signed-off-by: Yaniv Kaul <ykaul@redhat.com> Change-Id: Ied4210388976b6a7a2e91cc3de334534d6fef201 xlators/cluster/dht/src/dht-common.c: remove unneeded memset() Since we are going over the whole array anyway it is initialized properly. Only compile-tested! Signed-off-by: Yaniv Kaul <ykaul@redhat.com> Change-Id: Idc436d2bd0563b6582908d7cbebf9dbc66a42c9a xlators/cluster/ec/src/ec-helpers.c: remove unneeded memset() Since we are going over the whole array anyway it is initialized properly. Only compile-tested! Signed-off-by: Yaniv Kaul <ykaul@redhat.com> Change-Id: I81bf971f7fcecb4599e807d37f426f55711978fa xlators/mgmt/glusterd/src/glusterd-volgen.c: remove some memset() I don't think there's a need for any of them. Only compile-tested! Signed-off-by: Yaniv Kaul <ykaul@redhat.com> Change-Id: I476ea59ba53546b5153c269692cd5383da81ce2d xlators/mgmt/glusterd/src/glusterd-geo-rep.c: read() in 4K blocks The current 1K seems small. 4K is usually better (in Linux). Also remove a memset() that I don't think is needed between reads. Only compile-tested! Signed-off-by: Yaniv Kaul <ykaul@redhat.com> Change-Id: I5fb7950c92d282948376db14919ad12e589eac2b xlators/storage/posix/src/posix-{gfid-path|inode-fd-ops}.c: remove memset() before sys_*xattr() functions. I don't see a reason to memset the array sent to the functions sys_llistxattr(), sys_lgetxattr(), sys_lgetxattr(), sys_flistxattr(), sys_fgetxattr(). (Note: it's unclear to me why we are calling sys_*txattr() functions with XATTR_VAL_BUF_SIZE-1 size instead of XATTR_VAL_BUF_SIZE ). Only compile-tested! Change-Id: Ief2103b56ba6c71e40ed343a93684eef6b771346 updates: bz#1193929 Signed-off-by: Yaniv Kaul <ykaul@redhat.com>
* cluster/afr: Coverity fixes in afrkarthik-us2018-08-292-3/+5
| | | | | | | | | | | | | | | Fixes the deadcode issue in "afr-common.c" and null pointer dereference isse in "afr-dir-read.c". CIDs: 1395160, 1389018 Scan details: https://scan6.coverity.com/reports.htm#v42418/p10714/fileInstanceId=85017760&defectInstanceId=25877740&mergedDefectId=1395160 https://scan6.coverity.com/reports.htm#v42418/p10714/fileInstanceId=85017734&defectInstanceId=25877951&mergedDefectId=1389018 Change-Id: I65dff57305aa3ae43544be5353f801d761193e97 updates: bz#789278 Signed-off-by: karthik-us <ksubrahm@redhat.com>
* multiple files: move from strlen() to sizeof()Yaniv Kaul2018-08-293-10/+10
| | | | | | | | | | | | | | | {glusterfsd|glusterfsd-mgmt|quota-common-utils|xlator|tier|stripe}.c tools/setgfid2path/src/main.c xlators/cluster/afr/src/afr-inode-read.c {glusterfs-acl|glusterfs}.h For const strings, just do compile time size calc instead of runtime. Compile-tested only! Change-Id: I303684b1ff29b05c10126fb1057f507e404ced07 updates: bz#1193929 Signed-off-by: Yaniv Kaul <ykaul@redhat.com>
* cluster/afr: Delegate metadata heal with pending xattrs to SHDPranith Kumar K2018-08-283-38/+47
| | | | | | | | | | | | | | | | | | | Problem: When metadata-self-heal is triggered on the mount, it blocks lookup until metadata-self-heal completes. But that can lead to hangs when lot of clients are accessing a directory which needs metadata heal and all of them trigger heals waiting for other clients to complete heal. Fix: Only when the heal is needed but the pending xattrs are not set, trigger metadata heal that could block lookup. This is the only case where different clients may give different metadata to the clients without heals, which should be avoided. Updates bz#1622821 Change-Id: I6089e9fda0770a83fb287941b229c882711f4e66 Signed-off-by: Pranith Kumar K <pkarampu@redhat.com>
* cluster/ec: Prevent a possible out-of-bounds readVijay Bellur2018-08-271-0/+1
| | | | | | | | | | | Addresses CID 1370939 In ec_code_x64_epilog(), there is a possibility of reading from an incorrect index of ec_code_x64_regmap array Change-Id: Ib8a228bbe13631188343634b2bde5919cdaab5a4 Updates: bz#789278 Signed-off-by: Vijay Bellur <vbellur@redhat.com>
* multiple files: move from strlen() to sizeof()Yaniv Kaul2018-08-254-8/+8
| | | | | | | | | | | | {ec-heal|ec-combine|ec-helpers|ec-inode-read}.c For const strings, just do compile time size calc instead of runtime. Compile-tested only! Change-Id: If92ba0a7a20f64b898d01c6e3b6708190ca93e04 updates: bz#1193929 Signed-off-by: Yaniv Kaul <ykaul@redhat.com>
* stripe: FORWARD_NULL coverity fixShwetha Acharya2018-08-241-6/+9
| | | | | | | | | | | | Problem: frame could be NULL. Solution: Added condition checks to avoid NULL pointer dereferencing. CID: 1124478, 1124501, 1124504, 1124510 BUG: 789278 Change-Id: I5c81d912102a7e672386db3fdb820f883d08666f Signed-off-by: Shwetha Acharya <shwetha174@gmail.com>
* dht/switch: Fix coverity issuesShyamsundarR2018-08-241-4/+19
| | | | | | | | | | | | | | | | CID: 1124779: dup_childs not freed in an err path CID: 1382398: option_string leaked CID: 1382389: memcpy may cause string overflow Also, fixed up NULL termination for the string Potential use after free (or double free in this case) as below, (link split into multiple lines) https://download.gluster.org/pub/gluster/glusterfs/static-analysis/ master/glusterfs-coverity/2018-08-22-0ebaa9c6/html/ 1/427switch.c.html#error Change-Id: I76681af6a8091666918a3d5dff30a152a7b97905 Updates: bz#789278 Signed-off-by: ShyamsundarR <srangana@redhat.com>
* afr: common thin-arbiter functionsRavishankar N2018-08-236-12/+173
| | | | | | | | | | | | | | ...that can be used by client and self-heal daemon, namely: afr_ta_post_op_lock() afr_ta_post_op_unlock() Note: These are not yet consumed. They will be used in the write txn changes patch which will introduce 2 domain locking. updates: bz#1579788 Change-Id: I636d50f8fde00736665060e8f9ee4510d5f38795 Signed-off-by: Ravishankar N <ravishankar@redhat.com>
* cluster/dht: coverity fixesN Balachandran2018-08-212-12/+14
| | | | | | | | | Fixes 1133997, 1370910, 1382387, 1382444, 1394635 Change-Id: Ie63ad47abd5519b9b9536da26b61ed4c9eaf2c75 updates: bz#789278 Signed-off-by: N Balachandran <nbalacha@redhat.com>
* performance/readdir-ahead: keep stats of cached dentries in sync with ↵Krutika Dhananjay2018-08-181-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | modifications PROBLEM: Stats of dentries that are readdirp'd ahead can become stale due to fops like writes, truncate etc that modify the file pointed by dentries. When a readdir is finally wound at offset corresponding to these entries, the iatts that are returned to the application come from readdir-ahead's cache, which are stale by now. This problem gets further aggravated when caching translators/modules cache and continue to serve this stale information. FIX: * Store the iatt in context of the inode pointed by dentry. * Whenever the inode pointed by dentry undergoes modification, in cbk of modification fop, update the iatt stored in inode-ctx to reflect the modification. * When serving a readdirp response from application, update iatts of dentries with the iatts stored in the context of inodes pointed by these dentries. * Some fops don't have valid iatts in their responses. For eg., write response whose data is still cached in write-behind will have zeroed out stat. In this case keep only ia_type and ia_gfid and reset rest of the iatt members to zero. - fuse-bridge in this case just sends "entry" information back to kernel and attr is not sent. - gfapi sets entry->inode to NULL and zeroes out the entire stat * There is one tiny race between the entry creation and a readdirp on its parent dir, which could cause the inode-ctx setting and inode ctx reading to happen on two different inode objects. To prevent this, when entry->inode doesn't eqaul to linked_inode, - fuse-bridge is made to send only "entry" information without attributes - gfapi sets entry->inode to NULL and zeroes out the entire stat. Change-Id: Ia27ff49a61922e88c73a1547ad8aacc9968a69df BUG: 1390050 Updates: bz#1390050 Signed-off-by: Krutika Dhananjay <kdhananj@redhat.com> Signed-off-by: Raghavendra G <rgowdapp@redhat.com>
* cluster/ec: FORWARD_NULL coverity fixSunil Kumar Acharya2018-08-162-1/+5
| | | | | | | | | | | Fixing FORWARD_NULL coverify errors with EC. CID: 1394650 BUG: 789278 Change-Id: I52c99dac3483ca31a86cd7e3a959d4010b195f32 updates: bz#789278 Signed-off-by: Sunil Kumar Acharya <sheggodu@redhat.com>
* cluster/dht: Fixed rebalanced filesN Balachandran2018-08-141-1/+1
| | | | | | | | | An error caused skipped files to be counted as rebalanced files. Change-Id: I02333f099fb8b73ba953f41a2922021a1e4da7be fixes: bz#1615474 Signed-off-by: N Balachandran <nbalacha@redhat.com>