Possible to save unclean object database to file during replay

[abitmore]

During replay, undo_db is disabled, that said, when there is an exception thrown, changes made to object database won't rewound, which means the in-memory object database would be unclean.

When caught the exception, the node will dump object database from memory to disk. That means the on-disk object database would be unclean.

On next (and future) startup, if does not specify --replay-blockchain, the on-disk object database would be considered clean and be loaded into memory directly.

Related code:

bitshares-core/programs/witness_node/main.cpp

Lines 275 to 287 in 51e6c79

	} catch( const fc::exception& e ) {
	// deleting the node can yield, so do this outside the exception handler
	unhandled_exception = e;
	}
	if (unhandled_exception)
	{
	elog("Exiting with error:\n${e}", ("e", unhandled_exception->to_detail_string()));
	node->shutdown();
	delete node;
	return 1;
	}
	}

bitshares-core/libraries/app/application.cpp

Lines 1103 to 1109 in 51e6c79

	void application::shutdown()
	{
	if( my->_p2p_network )
	my->_p2p_network->close();
	if( my->_chain_db )
	{
	my->_chain_db->close();

CORE TEAM TASK LIST

• Evaluate / Prioritize Feature Request
• Refine User Stories / Requirements
• Define Test Cases
• Design / Develop Solution
  • Assigned: @cogutvalera
  • Estimating: TBD hours
• Perform QA/Testing
• Update Documentation

[pmconrad]

Nice catch.

[cogutvalera]

@ryanRfox I want to claim this issue if possible please

[ryanRfox]

Yes, @cogutvalera I will assign you. Please provide an estimate and the Core Team will review it.

[ryanRfox]

@abitmore I added this to 201810 Feature Release because it is labeled a bug and wanted some additional visibility of its progress. If it's not complete by release time, we can move it back to Future Non-Consensus (please leave it in Community Claims)

[cogutvalera]

@ryanRfox my estimation for this issue is approximately about 2 hours

[abitmore]

@cogutvalera I'm surprised. I didn't think it can be done in 2 hours. Please describe how to fix, and analyse pros and cons.

[cogutvalera]

@abitmore maybe I'm wrong with my estimation, very wrong, please help me to estimate this issue more correctly enough if you can.

My thoughts were that we can use something like next snippet of code after this line here https://github.com/bitshares/bitshares-core/blob/develop/programs/witness_node/main.cpp#L157

if (fc::exists(data_dir))
         fc::create_directories( data_dir / "object_database" / "lock" );

and here in the start of flush method we can use something like next code
https://github.com/bitshares/bitshares-core/blob/develop/libraries/db/object_database.cpp#L71

if( fc::exists( _data_dir / "object_database" / "lock" ) ) {
      fc::remove_all( _data_dir / "object_database" / "lock" );
      return;
   }

Are my thoughts wrong ? Or is it a good direction or not so ?

Thank you very much !

[cogutvalera]

My idea is simple but maybe I am completely wrong with it. Still researching different possibilities. My idea: not to flush object database in event of exception, also need to add check if it was in reply mode or not, but I hope you understand my idea correctly.

Thanks !

[abitmore]

Thanks. Perhaps it's not as hard as I thought. I think we need to add a parameter to shutdown() function to tell the object database whether the state is clean, to decide whether to dump it to disk.

[cogutvalera]

Wow, you are absolutely right about parameter in shutdown ! Did not thought yet about it. Nice idea ! Thanks a lot !

[cogutvalera]

At the beginning I thought the same as @abitmore that this issue is much harder but later after some researches I was surprised too that it is much easier than I have been expected.

[ryanRfox]

@cogutvalera and @abitmore did you two come to consensus about the estimate (2 hours)? I've assigned to @cogutvalera now and leave the estimate as TBD until I hear from you both. Thanks

[abitmore]

@ryanRfox depends on what's done. IMHO 2 hours is not sufficient.

[cogutvalera]

@ryanRfox I'm absolutely agree with @abitmore

[cogutvalera]

guys check please PR #1311

[pmconrad]

Tried to reproduce but failed.
Apparently fixed by 7c3968b in #1529 .