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.

Sign up for GitHub

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

IS-450 Reload rotation timestamp on each isExitTime request #1757

Merged
merged 11 commits into from
Jan 9, 2024
Merged

IS-450 Reload rotation timestamp on each isExitTime request #1757

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
e725930
Reload rotation timestamp in isTimeToRotate
badrogger Dec 14, 2023
5d8a88d
Rename getRotationTimestamp -> rotationTimestamp
badrogger Dec 14, 2023
85c1b31
Add logs
badrogger Dec 19, 2023
dd6fdae
Fix Net tests
badrogger Dec 21, 2023
f59f486
Merge branch 'v3.18.0' into reload-rotation-timestamp
badrogger Jan 3, 2024
b3f1c07
Handle malformed/missing rotationInfoFile
badrogger Jan 3, 2024
573f3c0
Rename rotation.txt -> rotation.json
badrogger Jan 4, 2024
1600857
Add error logger in InstanceMonitor
badrogger Jan 4, 2024
1785847
Rename m_info_logger -> m_infoLogger
badrogger Jan 5, 2024
53ecf1e
Merge branch 'v3.18.0' into reload-rotation-timestamp
DmytroNazarenko Jan 5, 2024
9ce9d5c
Merge branch 'v3.18.0' into reload-rotation-timestamp
badrogger Jan 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 9 additions & 10 deletions libethereum/InstanceMonitor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,23 +46,22 @@ void InstanceMonitor::initRotationParams( uint64_t _finishTimestamp ) {
std::ofstream rotationInfoFile( m_rotationInfoFilePath.string() );
rotationInfoFile << rotationJson;

m_finishTimestamp = _finishTimestamp;
LOG( m_logger ) << "Set rotation time to " << m_finishTimestamp;
LOG( m_logger ) << "Set rotation time to " << _finishTimestamp;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add try catch

}

bool InstanceMonitor::isTimeToRotate( uint64_t _finishTimestamp ) {
bool InstanceMonitor::isTimeToRotate( uint64_t _blockTimestamp ) const {
if ( !fs::exists( m_rotationInfoFilePath ) ) {
return false;
}
return m_finishTimestamp <= _finishTimestamp;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's better to write information about rotation into logs

return rotationTimestamp() <= _blockTimestamp;
}

void InstanceMonitor::restoreRotationParams() {
if ( fs::exists( m_rotationInfoFilePath ) ) {
std::ifstream rotationInfoFile( m_rotationInfoFilePath.string() );
auto rotationJson = nlohmann::json::parse( rotationInfoFile );
m_finishTimestamp = rotationJson["timestamp"].get< uint64_t >();
}
uint64_t InstanceMonitor::rotationTimestamp() const {
std::ifstream rotationInfoFile( m_rotationInfoFilePath.string() );
auto rotationJson = nlohmann::json::parse( rotationInfoFile );
auto timestamp = rotationJson["timestamp"].get< uint64_t >();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add try catch to handle the case of corrupt file

LOG( m_logger ) << "Rotation scheduled for " << timestamp;
return timestamp;
}

void InstanceMonitor::reportExitTimeReached( bool _reached ) {
Expand Down
13 changes: 4 additions & 9 deletions libethereum/InstanceMonitor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,23 +36,18 @@ class InstanceMonitor {
public:
explicit InstanceMonitor( const boost::filesystem::path& _rotationInfoFileDirPath,
std::shared_ptr< StatusAndControl > _statusAndControl = nullptr )
: m_finishTimestamp( 0 ),
m_rotationInfoFilePath( _rotationInfoFileDirPath / rotation_info_file_name ),
: m_rotationInfoFilePath( _rotationInfoFileDirPath / rotation_info_file_name ),
m_statusAndControl( _statusAndControl ) {
restoreRotationParams();
reportExitTimeReached( false );
}
void prepareRotation();
void initRotationParams( uint64_t _finishTimestamp );
bool isTimeToRotate( uint64_t _finishTimestamp );
bool isTimeToRotate( uint64_t _blockTimestamp ) const;

protected:
void restoreRotationParams();
[[nodiscard]] uint64_t finishTimestamp() const { return m_finishTimestamp; }

[[nodiscard]] uint64_t rotationTimestamp() const;
[[nodiscard]] fs::path rotationInfoFilePath() const { return m_rotationInfoFilePath; }

uint64_t m_finishTimestamp;
const fs::path m_rotationInfoFilePath;
std::shared_ptr< StatusAndControl > m_statusAndControl;

Expand All @@ -61,5 +56,5 @@ class InstanceMonitor {
void reportExitTimeReached( bool _reached );

private:
dev::Logger m_logger{ createLogger( dev::VerbosityInfo, "instance-monitor" ) };
mutable dev::Logger m_logger{ createLogger( dev::VerbosityInfo, "instance-monitor" ) };
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mutable is not needed here

};
13 changes: 8 additions & 5 deletions test/unittests/libethereum/InstanceMonitorTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,6 @@ class InstanceMonitorMock: public InstanceMonitor {
public:
explicit InstanceMonitorMock(fs::path const &rotationFlagFilePath, std::shared_ptr<StatusAndControl> statusAndControl) : InstanceMonitor(rotationFlagFilePath, statusAndControl) {};

uint64_t getFinishTimestamp() {
return this->finishTimestamp();
};

fs::path getRotationInfoFilePath() {
return this->rotationInfoFilePath();
}
Expand All @@ -32,6 +28,10 @@ class InstanceMonitorMock: public InstanceMonitor {
void removeFlagFileTest(){
this->reportExitTimeReached( false );
}

uint64_t getRotationTimestamp() const {
return this->rotationTimestamp();
}
};

class InstanceMonitorTestFixture : public TestOutputHelperFixture {
Expand Down Expand Up @@ -70,7 +70,7 @@ BOOST_AUTO_TEST_CASE( test_initRotationParams ) {
uint64_t ts = 100;
BOOST_REQUIRE( !fs::exists(instanceMonitor->getRotationInfoFilePath() ) );
instanceMonitor->initRotationParams(ts);
BOOST_CHECK_EQUAL(instanceMonitor->getFinishTimestamp(), ts);
BOOST_CHECK_EQUAL(instanceMonitor->getRotationTimestamp(), ts);

BOOST_REQUIRE( fs::exists(instanceMonitor->getRotationInfoFilePath() ) );

Expand Down Expand Up @@ -98,6 +98,9 @@ BOOST_AUTO_TEST_CASE( test_isTimeToRotate_true ) {

instanceMonitor->initRotationParams(50);
BOOST_REQUIRE( instanceMonitor->isTimeToRotate( currentTime ) );

currentTime = 49;
BOOST_REQUIRE( !instanceMonitor->isTimeToRotate( currentTime ) );
}

BOOST_AUTO_TEST_CASE( test_rotation ) {
Expand Down
Loading