Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove fetching items in gf_cli_replace_brick(), gf_cli_reset_brick() #753

Closed
mykaul opened this issue Oct 28, 2019 · 5 comments
Closed
Labels

Comments

@mykaul
Copy link
Contributor

mykaul commented Oct 28, 2019

Those functions don't do anything with some of the items they fetch from the dictionary, so those dict_get... could be removed most likely. Perhaps they are used as safe-guards - but surely they are later fetched and checked anyway.

@mykaul
Copy link
Contributor Author

mykaul commented Oct 28, 2019

Also in gf_cli_add_brick()

@mykaul
Copy link
Contributor Author

mykaul commented Oct 28, 2019

gf_cli_clearlocks_volume_cbk() as well?

@gluster-ant
Copy link
Collaborator

A patch https://review.gluster.org/23810 has been posted that references this issue.

cli - removing fetch of unnecessary items.

Somem methods dict_get...(...) values and then not use them anywhere.
Removed found occurrences.

fixes: #753

Change-Id: Iaeb8f4cec18f76078f6b2f4e4bd6f9795a3467bc
Signed-off-by: Barak Sason Rofman bsasonro@redhat.com

@BarakSason
Copy link
Contributor

gerrit:
"Missing SpecApproved flag on Issue 753
Missing DocApproved flag on Issue 753"

Please update relevant labels so smoke will pass.

@amarts amarts added the Type:Bug label Dec 4, 2019
@gluster-ant
Copy link
Collaborator

A patch https://review.gluster.org/23810 has been posted that references this issue.

cli - removing fetch of unnecessary items.

Somem methods dict_get...(...) values and then not use them anywhere.
Removed found occurrences.

fixes: #753

Change-Id: Iaeb8f4cec18f76078f6b2f4e4bd6f9795a3467bc
Signed-off-by: Barak Sason Rofman bsasonro@redhat.com

gluster-ant pushed a commit that referenced this issue Dec 19, 2019
1. Move functions and structs to static
2. Use dictionary functions with fixed key length.
3. Reduce key length when not needed.
4. Use const char* for some messages.
5. Use fixed strings for some logs which is repeated in the code.
6. Remove redundant checks. Specifically, cli_to_glusterd() does
NULL checks already, so no need to do it before calling it.
7. Aligned some messages - not sure why they were cut over several
lines, but it made grep on the code harder.
8. Move dictionary fetching of values closer to where they are
actually used.

Overall, object size is ~4 smaller, hopefully without functional changes.

There's more that can be done, but as this is a very long (>10K lines)
file, I think it's enough for one change.
Specifically, some functions fetch values from the dictionary
without using it - this is a bit of a waste.
Filed #753 about it.

Change-Id: I31f88d94ab25398e00aef2ea84a8c4af9383b75b
updates: bz#1193929
Signed-off-by: Yaniv Kaul <ykaul@redhat.com>
amarts pushed a commit to amarts/glusterfs that referenced this issue Jan 1, 2020
1. Move functions and structs to static
2. Use dictionary functions with fixed key length.
3. Reduce key length when not needed.
4. Use const char* for some messages.
5. Use fixed strings for some logs which is repeated in the code.
6. Remove redundant checks. Specifically, cli_to_glusterd() does
NULL checks already, so no need to do it before calling it.
7. Aligned some messages - not sure why they were cut over several
lines, but it made grep on the code harder.
8. Move dictionary fetching of values closer to where they are
actually used.

Overall, object size is ~4 smaller, hopefully without functional changes.

There's more that can be done, but as this is a very long (>10K lines)
file, I think it's enough for one change.
Specifically, some functions fetch values from the dictionary
without using it - this is a bit of a waste.
Filed gluster/glusterfs#753 about it.

Change-Id: I31f88d94ab25398e00aef2ea84a8c4af9383b75b
updates: bz#1193929
Signed-off-by: Yaniv Kaul <ykaul@redhat.com>
amarts pushed a commit to amarts/glusterfs that referenced this issue Mar 24, 2020
1. Move functions and structs to static
2. Use dictionary functions with fixed key length.
3. Reduce key length when not needed.
4. Use const char* for some messages.
5. Use fixed strings for some logs which is repeated in the code.
6. Remove redundant checks. Specifically, cli_to_glusterd() does
NULL checks already, so no need to do it before calling it.
7. Aligned some messages - not sure why they were cut over several
lines, but it made grep on the code harder.
8. Move dictionary fetching of values closer to where they are
actually used.

Overall, object size is ~4 smaller, hopefully without functional changes.

There's more that can be done, but as this is a very long (>10K lines)
file, I think it's enough for one change.
Specifically, some functions fetch values from the dictionary
without using it - this is a bit of a waste.
Filed gluster/glusterfs#753 about it.

Change-Id: I31f88d94ab25398e00aef2ea84a8c4af9383b75b
updates: bz#1193929
Signed-off-by: Yaniv Kaul <ykaul@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants