Logbuffer metadata extra fields #1700
Conversation
pveentjer
changed the title
| /** | ||
| * Offset within the log metadata where the term offset is stored. | ||
| */ | ||
| public static final int LOG_TERM_OFFSET_OFFSET; |
There was a problem hiding this comment.
A natural place to store this information would the default frame header, i.e.:
defaultDataHeader.sessionId(sessionId).streamId(streamId).termId(initialTermId).termOffset(termOffset);
storeDefaultFrameHeader(logMetaData, defaultDataHeader);There was a problem hiding this comment.
Ok. I'll set the term offset on the default frame header and will drop it from the LogBufferDescriptor
| throw new Error("Bad alignment: offset=" + offset); | ||
| } | ||
|
|
||
| LOG_SOCKET_RCVBUF_LENGTH_OFFSET = offset; |
There was a problem hiding this comment.
Let's try to use available space after the page_size field (LOG_PAGE_SIZE_OFFSET + SIZE_OF_INT) where we still have 36 bytes before the default header starts.
That should be enough to fit 6 ints or 5 ints + 6 booleans.
|
@pveentjer Please define new fields in the same order and using the same names as defined in the |
…stem_tests_c_media_driver doesn't fail
pveentjer
changed the title
| @@ -71,6 +71,7 @@ typedef struct aeron_logbuffer_metadata_stct | |||
| uint8_t signal_eos; | |||
| uint8_t spies_simulate_connection; | |||
| uint8_t tether; | |||
| uint8_t pad_end[32]; // Padding to align the structure size to 512 bytes (9x AERON_CACHE_LINE_LENGTH) | |||
There was a problem hiding this comment.
Why add trailing padding? The entire metadata section is 4KB and is cache line aligned. We are going to read only the fields we need, i.e. consume up to 480 bytes.
There was a problem hiding this comment.
The original code also had padding at the end of the struct:
uint8_t pad3[(AERON_CACHE_LINE_LENGTH) - (7 * sizeof(int32_t))];
My guess is that it was done to prevent false sharing.
There was a problem hiding this comment.
I had another look. I think I understand what is happening.
The padding is needed to position the defaultHeader. It isn't needed to prevent false sharing.
Since the empty space (the pad3) is now filled with the extra fields, the padding isn't relevant any longer.
There was a problem hiding this comment.
Yes, so no extra padding is needed. The definition on the C side was correct after the 9eb689f commit.
| @@ -71,6 +71,7 @@ typedef struct aeron_logbuffer_metadata_stct | |||
| uint8_t signal_eos; | |||
| uint8_t spies_simulate_connection; | |||
| uint8_t tether; | |||
| uint8_t pad_end[32]; // Padding to align the structure size to 512 bytes (9x AERON_CACHE_LINE_LENGTH) | |||
There was a problem hiding this comment.
Yes, so no extra padding is needed. The definition on the C side was correct after the 9eb689f commit.
aeron-client/src/main/java/io/aeron/logbuffer/LogBufferDescriptor.java
Outdated
Show resolved
Hide resolved
aeron-client/src/main/java/io/aeron/logbuffer/LogBufferDescriptor.java
Outdated
Show resolved
Hide resolved
aeron-client/src/main/java/io/aeron/logbuffer/LogBufferDescriptor.java
Outdated
Show resolved
Hide resolved
aeron-client/src/main/java/io/aeron/logbuffer/LogBufferDescriptor.java
Outdated
Show resolved
Hide resolved
aeron-client/src/main/java/io/aeron/logbuffer/LogBufferDescriptor.java
Outdated
Show resolved
Hide resolved
aeron-client/src/main/java/io/aeron/logbuffer/LogBufferDescriptor.java
Outdated
Show resolved
Hide resolved
aeron-client/src/main/java/io/aeron/logbuffer/LogBufferDescriptor.java
Outdated
Show resolved
Hide resolved
aeron-driver/src/main/java/io/aeron/driver/DriverConductor.java
Outdated
Show resolved
Hide resolved
aeron-driver/src/main/java/io/aeron/driver/DriverConductor.java
Outdated
Show resolved
Hide resolved
Adding extra fields to the metadata section of the logbuffers for Aeron Insights.
This PR is a replacement for the following one:
#1697
Which isn't going to be merged because it will break backwards compatibility.