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

Log when --targetgaslimit parameter is in effect #17076

Closed
wbt opened this issue Jun 25, 2018 · 9 comments
Closed

Log when --targetgaslimit parameter is in effect #17076

wbt opened this issue Jun 25, 2018 · 9 comments

Comments

@wbt
Copy link
Contributor

wbt commented Jun 25, 2018

System information

Geth version: 1.8.8-stable
OS & Version: Windows 10

Setup

I have a private PoA network with one validator node (one node total in this simple network). I tried using single quotes and double quotes for the parameter and it didn’t matter. I have a relatively high value for the gasLimit parameter in genesis.json (namely 6721975) and pass in a similarly high value for the --targetgaslimit parameter when starting geth.

For this example, I have no transactions being broadcast to the network, just mining of nothingness.

In another window, I use geth attach http://<network IP and port> successfully, and use eth.getBlock(3).gasLimit or similar to get block-specific gas limit information (in this case for block 3).

Looking at the gasLimit parameter of the block, I see that it's dropping with each block. If I start with a much smaller gasLimit in the genesis block, the limit rises with each block. The gas limit stabilizes at 4712388 (not 3141592, as in Issue #2185), a value which is found in /params/protocol_params.go:28, set as the initial value of TargetGasLimit six lines earlier.

It looks like the value from the flag is supposed to be incorporated in SetupNetwork, cmd/utils/flags.go:1181-85, called at /cmd/geth/main.go:214.

If there is some issue in the command to start geth, and this command-line flag follows that issue, this parameter is ignored. The user can then see that eth.getBlock(n).gasLimit does not approach --targetgaslimit as n increases, but rather approaches 4712388, and this is confusing behavior.

Expected behaviour

In geth startup, an INFO log message is generated printing the target gas limit for that node for that mining startup.

Actual behaviour

No message is generated, so if the parameter used by Geth differs from what appears to have been passed in the geth start command, the user has some confusing debugging ahead.

The core enhancement request here is to add a line of logging at geth startup to indicate the value of the target gas limit parameter.

@wbt wbt changed the title --targetgaslimit parameter ignored? Log when --targetgaslimit parameter is in effect Jun 25, 2018
@karalabe
Copy link
Member

Could you please paste the entire command you're using to start geth?

@wbt
Copy link
Contributor Author

wbt commented Jun 26, 2018

@karalabe As you suspected, I found that the root cause for the issue motivating my original post was an issue in the command, which had been copied and pasted into an e-mail client that has "smart" formatting and converted one of the double-hyphen pairs into a longer dash, which when copied and pasted into terminal pastes as nothing. Omitting those double dashes caused that command-line flag and everything after it (including --targetgaslimit) to be ignored, with no error message, and behavior (in the case of --targetgaslimit, silently) consistent with the defaults.

I found that as the core cause of the problem shortly after the first post of this Issue and then edited to focus the request just on logging. Having a log message output at the start showing the target gas limit in effect (whether using the default or one interpreted from the command-line arguments) would have made debugging a lot easier and faster.

@wbt
Copy link
Contributor Author

wbt commented Jun 26, 2018

To be more specific, the parameter lacking the double hyphen was ipcdisable, which immediately followed --mine. This came fairly late in a long command starting with geth and a space-separated list of --paramname value pairs, so seeing something like --mine ipcdisable --gasprice "1000000000" --targetgaslimit "6721975" seemed to fit that format. (These are example values for illustration.)

No command errors jumped out based on a review of just the formatting, where ipcdisable appeared as a mode/value for the --mine parameter (it isn't, but that doesn't become clear until more detailed geth specific knowledge is more carefully applied), and that portion of the command did not get as much actively focused attention until later in the debugging process. Startup log messages revealing that the ipcdisable parameter was getting ignored did help, once attention got to that issue, but it would have been solved a lot faster with a startup log message disclosing the value being used for targetgaslimit.

@stale
Copy link

stale bot commented Jun 27, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@wbt
Copy link
Contributor Author

wbt commented Jun 28, 2019

I would still like to see this logging added.

@stale stale bot removed the status:inactive label Jun 28, 2019
@wbt
Copy link
Contributor Author

wbt commented Jun 28, 2019

PR 17089 attempted to address this issue, but was not autolinked.

@wbt
Copy link
Contributor Author

wbt commented Nov 27, 2019

I'm not quite sure why this was closed; @adamschmideg could you please add a few words on that?

@adamschmideg
Copy link
Contributor

I understood you solved your original issue. Please reopen it if you think I'm wrong. Also consider if you could add this as an idea to #18243 which is trying to address similar issues at a more general level.

@wbt
Copy link
Contributor Author

wbt commented Dec 2, 2019

@adamschmideg Thanks for responding.

No, I don't see any evidence that the issue has been addressed. I don't see that there's a new line of logging at geth startup to indicate the value of the target gas limit parameter.

That additional logging is the core of the request above and was reinforced over a year later. I even offered a code contribution attempting to address this specific issue and reinforced there the request for that additional log line.

Even if other commits make it less likely people will encounter issues like what prompted me to find this one, there will still be cases where knowing what target gas limit parameter is in effect will be helpful. Adding that one log line also seems like such an easy thing to do, probably even easier than the time spent on composing discussion responses like this one.

I still think this would be a good idea to help someone better understand what is going on in the code and save hours of diagnostic debugging if that doesn't match their expectations.

I also do not have the privilege of reopening issues in this repo (feel free to change that if you think I should) and think that opening dupes of issues closed with no clear reason would not be a good service to the repo/community.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants